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

Control Power and Source Strength #1001

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

lewisgross1296
Copy link
Contributor

@lewisgross1296 lewisgross1296 commented Dec 4, 2024

Closes #1000 (woohoo 🎉 🎉1000 issues+PRs!!)

@lewisgross1296
Copy link
Contributor Author

lewisgross1296 commented Dec 5, 2024

@gonuke @pshriwise
Any idea why pyne failed due to (unrelated) changes in OpenMCCellAverageProblem?

-- Submodule update
Submodule 'src/pyne/pyne' (https://github.com/pyne/pyne.git) registered for path 'src/pyne/pyne'
Cloning into '/data/civet0/build/cardinal/contrib/DAGMC/src/pyne/pyne'...
fatal: unable to access 'https://github.com/pyne/pyne.git/': Could not resolve host: github.com
fatal: clone of 'https://github.com/pyne/pyne.git' into submodule path '/data/civet0/build/cardinal/contrib/DAGMC/src/pyne/pyne' failed
Failed to clone 'src/pyne/pyne'. Retry scheduled
Cloning into '/data/civet0/build/cardinal/contrib/DAGMC/src/pyne/pyne'...
fatal: unable to access 'https://github.com/pyne/pyne.git/': Could not resolve host: github.com
fatal: clone of 'https://github.com/pyne/pyne.git' into submodule path '/data/civet0/build/cardinal/contrib/DAGMC/src/pyne/pyne' failed
Failed to clone 'src/pyne/pyne' a second time, aborting
CMake Error at CMakeLists.txt:38 (message):
  git submodule update --init --recursive failed with 1, please checkout
  submodules


-- Configuring incomplete, errors occurred!
-- Cache values
CMAKE_BUILD_TYPE:STRING=Release
CMAKE_INSTALL_PREFIX:PATH=/data/civet0/build/cardinal/install
GIT_SUBMODULE:BOOL=ON
make: *** [/data/civet0/build/cardinal/config/dagmc.mk:3: /data/civet0/build/cardinal/build/DAGMC/Makefile] Error 1

ERROR: exiting with code 2

@gonuke
Copy link

gonuke commented Dec 5, 2024

This may be related to scaling/DAGMC#968 that was driven by a request from @nuclearkevin

@gonuke
Copy link

gonuke commented Dec 5, 2024

Notably - CIVET doesn't have access to the outside world. So something more substantial must have changed than you think.

@lewisgross1296
Copy link
Contributor Author

It does look like some of the other actions failed for other reaons. I'll look into those too

@nuclearkevin
Copy link
Contributor

nuclearkevin commented Dec 5, 2024

@lewisgross1296 The failures in Test, Test DAGMC, and Documentation are caused by an update to the CIVET test recipes that Cardinal uses (which removed pyne as a Cardinal dependency since we don't use the UWUW). As CIVET no longer initializes DAGMC's pyne submodule during the cloning phase, DAGMC attempts to clone it when building. This causes errors since the CIVET build boxes don't have internet access while compiling code, causing git failures when attempting to clone pyne. Once the change @gonuke made (which only clones pyne if UWUW is enabled with a compile flag) in svalinn/DAGMC#968 is merged, we'll update Cardinal's DAGMC submodule to fix these test failures.

The other failures you're encountering are compiler errors. You're attempting to initialize a constant reference outside of the OpenMCCellAverageProblem initializer list, which is forbidden in C++. Moving the assignments to the initializer list should resolve those.

@lewisgross1296
Copy link
Contributor Author

I finally got this to compile, but wanted to ask for feedback.

We are changing _power and _source_strength to be declared as const references in include/base/OpenMCProblemBase.h. As pointers, the existing devel branch assigns _power or _source_strength in the src/base/OpenMCProblemBase.C constructor dependent on the value of _run_mode (and checks to make sure only one of the two is assigned in the [Problem] block).

switch (_run_mode)
{
case openmc::RunMode::EIGENVALUE:
{
// Jumping through hoops to see if we're going to add tallies down the line.
if (tally_actions.size() > 0)
{
checkRequiredParam(params, "power", "running in k-eigenvalue mode");
_power = &getPostprocessorValue("power");
}
else
checkUnusedParam(params, "power", "no tallies have been added");
checkUnusedParam(params, "source_strength", "running in k-eigenvalue mode");
break;
}
case openmc::RunMode::FIXED_SOURCE:
{
if (tally_actions.size() > 0)
{
checkRequiredParam(params, "source_strength", "running in fixed source mode");
_source_strength = &getPostprocessorValue("source_strength");
}
else
checkUnusedParam(params, "source_strength", "no tallies have been added");

So now these assignments need to be removed from the OpenMCProblemBase constructor and converted to list initialization. However they are conditioned on _run_mode, but _run_mode doesn't get set until inside the constructor of OpenMCProblemBase. To get around that, I initialize as such

OpenMCProblemBase::OpenMCProblemBase(const InputParameters & params)
  : CardinalProblem(params),
    PostprocessorInterface(this),
    _verbose(getParam<bool>("verbose")),
    _power(openmc::settings::run_mode == openmc::RunMode::EIGENVALUE
               ? getPostprocessorValue("power")
               : 0),
    _source_strength(openmc::settings::run_mode == openmc::RunMode::FIXED_SOURCE
                         ? getPostprocessorValue("source_strength")
                         : 0),
    _reuse_source(getParam<bool>("reuse_source")),
    _specified_scaling(params.isParamSetByUser("scaling")),
    _scaling(getParam<Real>("scaling")),
    _skip_statepoint(getParam<bool>("skip_statepoint")),
    _fixed_point_iteration(-1),
    _total_n_particles(0)

Do others have opinions on setting the values to 0 dependent on run mode? I removed the assignments from the constructors, but kept all the other checks, so it should behave the same as before.

@moosebuild
Copy link
Collaborator

Job Documentation, step Sync to remote on ee04054 wanted to post the following:

View the site here

This comment will be updated on new commits.

@lewisgross1296
Copy link
Contributor Author

So it looks like despite compiling, there's still a larger issue. Apparently OpenMCCellAverageProblem::power is of type PostprocessorName, which is not allowed to be controllable.

non-controllable type 'PostprocessorName' for parameter 'Problem/power' marked controllable: non-controllable type 'PostprocessorName' for parameter '' marked controllable

At this point, I think this is a MOOSE design question. Tagging @loganharbour for input

@loganharbour
Copy link
Member

Yeah, this isn't how controllable parameters work. To control something, it needs to be a value. Not a name that points to something else. I might need to understand the goal a bit more.

@lewisgross1296
Copy link
Contributor Author

lewisgross1296 commented Dec 10, 2024

As of right now OpenMCCellAverageProblem has a parameter named power (how the user specifies the power used for tally normalization) and OpenMCCellAverageProblem stores it as Postoprocessor. I can't think of a reason that it needs to stay as a Postprocessor, though there may be some. I wanted to make it controllable so I can do load-following simulations with Cardinal. Should we re-think the approach for load-following or can we change the way we store power so that it can use the Controls system?

@aprilnovak
Copy link
Collaborator

power used to just be a const Real, but we modified it a few years ago to be a postprocessor so that it could be more easily set by another multiapp (e.g., point kinetics) for transients. See: #387

It's possible that we can achieve the same thing with a redesign -- I'm not married to postprocessors if there's a way to do this solely with controls.

@nuclearkevin
Copy link
Contributor

@lewisgross1296 A rather clunky workaround which will allow you to specify a controllable value of power/source_strength would be to create a ConstantFunction where you specify the power in value (which is controllable). You could then pass this to a FunctionValuePostprocessor, which you then use for power in your OpenMCCellAverageProblem.

I haven't tried this myself so I can't guarantee it'll work, but it's worth a shot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Control power and source strength
6 participants