Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix recording resources issue #15

Merged
merged 18 commits into from
Jun 27, 2023
Merged

Fix recording resources issue #15

merged 18 commits into from
Jun 27, 2023

Conversation

abachma2
Copy link
Collaborator

This PR fixes the broken functionality of the spent fuel not being recorded as separate resources from the fresh fuel. Merging this PR will close Issue #9. All tests are passing locally, the CI is still broken (see issue #12), to be updated in a later PR.

…urces

Conflicts:
	openmcyclus/DepleteReactor.py
The ReactorEvents table is created and some of the columns can
be written to it, but the column names are wrong. The spent fuel
resources are partially written to the Resources table, but only
up through the TimeCreated column. Not sure why the other
columns can't be written.
Needed to adjust the Quantity to a double. The table is full now.
The Parent 1 isn't correct, but I'm not sure how to get it to be
correct and match the state_id of the parent (i.e. before the
state id is bumped)
@abachma2 abachma2 self-assigned this Jun 19, 2023
@abachma2 abachma2 added Comp:Output This issue has to do with the output component of the code or document. (writing to databases, etc.) Difficulty:1-Beginner This issue does not require expert knowledge and may be a good issue for new contributors. Priority:2-Normal This work is important and should be completed ASAP. Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) Type:Feature New feature or feature request labels Jun 19, 2023
If fuel is to be transmuted at each cycle end, then the
spent fuel resources don't need to be recording each time fuel
is transmuted. This also captures all of the fuel that is
dischraged from a reactor, not just the transmuted fuel. This only has
an impact when a facility is decommissioned during a simulation
Specifying the decom_transmute_all in Cycamore means that
all of the fuel discharged from the reactor at decommissioning
gets transmuted, and thus recorded in the Resources database
Copy link
Contributor

@ZoeRichter ZoeRichter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - I'm not seeing any glaring issues in the code. The CI breaking is a concern, but from what I read in your PRs, it seems like you're aware of the issue, the cause is external to this PR, and you intend to address it in a separate PR, so in this instance I think it should be alright to merge.

@abachma2
Copy link
Collaborator Author

Thanks for the review @ZoeRichter. The contents of the PR don't rely on the other PR, so this is ready to be merged on my end. Is it ready to be merged on your end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Comp:Output This issue has to do with the output component of the code or document. (writing to databases, etc.) Difficulty:1-Beginner This issue does not require expert knowledge and may be a good issue for new contributors. Priority:2-Normal This work is important and should be completed ASAP. Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) Type:Feature New feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants