-
Notifications
You must be signed in to change notification settings - Fork 92
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
Allowing users to set powerDensity instead of power #1395
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Easier than I expected. Thanks!
self.r.core.p.power = ( | ||
self.powerFractions[cycle][timeNode] * self.cs["power"] | ||
) | ||
self.r.core.p.power = self.powerFractions[cycle][timeNode] * basicPower |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, this ends up being much easier than I expected. I thought we were going to have to replace calls to cs["power"]
all over the place. But because people are already used to the fact that power can change via powerFractions
, I guess they are used to always going to the operator to get the current power level.
I did find (at least) one instance of a direct call to cs["power"]
in an internal implementation of a custom operator class. So we'll have to fix that as well.
Co-authored-by: Chris Keckler <ckeckler@terrapower.com>
Co-authored-by: Chris Keckler <ckeckler@terrapower.com>
@john-science - one question I have is if having a parameter of power density should have a setter on it to be up to date with power in the run? Say the power changes as a parameter, should the reactor core's power density be auto derived and vice versa? Sorry for the late comment! |
Thanks @john-science! I was out last week, but this looks to be exactly what we'd need |
…t-mod-empty-validation * terrapower/main: (23 commits) Allowing users to set powerDensity instead of power (terrapower#1395) Fix assemNum parameter for uniform mesh reactor. (terrapower#1398) Update to black v22.6.0 (terrapower#1396) Fixing gallery example for new working Grids (terrapower#1394) Refactor armi.reactor.grids to wider submodule; Grids as ABC (terrapower#1373) Make SFP a child of reactor. (terrapower#1349) Move max assembly number out of global scope (terrapower#1383) Ensuring we throw an error on bad YAML files (terrapower#1358) Removing unused settings (terrapower#1393) Fixing some miscellaneous TODOs (terrapower#1392) Removing global plugin flags (terrapower#1388) Update reactivity coefficient parameters (terrapower#1355) Fixing ruff formatting of doc gallery examples (terrapower#1389) Fixing broken link (terrapower#1390) Removing unreachable code block (terrapower#1391) Removing unnecessary f-strings (terrapower#1387) Updating documentation dependencies for Python 3.11 (terrapower#1378) Adding a little code coverage to the CLIs (terrapower#1371) Remove coveragepy-lcov dependency during CI. (terrapower#1382) Reconnect logger after disconnecting in test. (terrapower#1381) ...
What is the change?
The user can now choose to set
powerDensity
, instead ofpower
in the settings.The
powerDensity
is the total thermal power of the Core in Watts per grams of heavy metal mass.Why is the change being made?
@drewj-usnctech requested this feature, which will close #1365
He says it would be useful in practice for a lot of engineers real-world workflows. And as it appears to be a small/simple change, I think supporting more engineers is better.
Checklist
doc/release/0.X.rst
) are up-to-date with any important changes.doc
folder.setup.py
.