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

Change b.axialExpTargetComponent to block parameter #1256

Merged
merged 9 commits into from
May 5, 2023

Conversation

albeanth
Copy link
Member

@albeanth albeanth commented May 3, 2023

Description

Downstream analysis identified an issue where the blueprints specified axial expansion target component was unable to be used with database loads (i.e., restarts and snapshot runs). This PR changes the Block class instance variable, b.axialExpTargetComponent, to a registered block parameter that gets written to the db.

When a standard run to executed, the axial expansion target components for each block are stored as block parameters and written to the DB. When used for a DB load case, they are directly read off of the database.


Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any bug fixes or new features.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

@albeanth albeanth added the bug Something is wrong: Highest Priority label May 3, 2023
@albeanth albeanth requested review from mgjarrett and keckler May 3, 2023 20:18
@albeanth albeanth marked this pull request as draft May 3, 2023 20:52
@albeanth
Copy link
Member Author

albeanth commented May 3, 2023

Found an issue with this. Will reopen when ready.

@albeanth albeanth marked this pull request as ready for review May 3, 2023 21:28
@albeanth
Copy link
Member Author

albeanth commented May 3, 2023

Found an issue with this. Will reopen when ready.

Note to reviewers: The issue was that originally, b.p.axialExpTargetComponent was storing the Component directly. That caused problems when writing out to the DB. To resolve this, I changed it to store the Component.name instead. I was able to confirm that by making this change and changing the default value from None to "", b.p.axialExpTargetComponent is written as expected. It should be represented the same way b.p.type is represented.

@albeanth
Copy link
Member Author

albeanth commented May 4, 2023

Coverage failure seems to be coverage spray....

@john-science

@albeanth albeanth marked this pull request as draft May 4, 2023 22:21
@albeanth
Copy link
Member Author

albeanth commented May 4, 2023

Converting to draft while this waits it's turn in line within a downstream ARMI app.

@albeanth albeanth marked this pull request as ready for review May 5, 2023 16:31
@albeanth albeanth merged commit b9bc69d into terrapower:main May 5, 2023
9 checks passed
@albeanth albeanth deleted the axialExpTargetComp2Param branch May 5, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants