OpenMC initialization action#1302
OpenMC initialization action#1302JoffreyDorville wants to merge 7 commits intoneams-th-coe:develfrom
Conversation
|
Job Documentation, step Sync to remote on 8fddd46 wanted to post the following: View the site here This comment will be updated on new commits. |
|
Job Coverage, step Generate coverage on 8fddd46 wanted to post the following: CoverageInconsistent report tags were found between the head and base reports. Inconsistent tags: This comment will be updated on new commits. |
meltawila
left a comment
meltawila
left a comment
There was a problem hiding this comment.
I only have minor comments below, and I agree we should protect against multiple calls of this action. Cardinal shouldn't initialize OpenMC more than once
| openmc::DEFAULT_IFP_N_GENERATION, | ||
| "The number of generations to use with the method of iterated fission probabilities."); | ||
| params.addParam<FileName>( | ||
| "xml_directory", "./", "The directory in which to look for OpenMC XML files."); |
| "ifp_generations", | ||
| openmc::DEFAULT_IFP_N_GENERATION, | ||
| "The number of generations to use with the method of iterated fission probabilities."); | ||
| params.addParam<FileName>( |
| "ifp_generations", | ||
| openmc::DEFAULT_IFP_N_GENERATION, | ||
| "The number of generations to use with the method of iterated fission probabilities."); | ||
| params.addParam<FileName>( |
There was a problem hiding this comment.
Should this be removed as well?
| OpenMCInitAction::validParams() | ||
| { | ||
| InputParameters params = MooseObjectAction::validParams(); | ||
| params.addParam<std::string>("type", "Problem type"); |
There was a problem hiding this comment.
Isn't this redundant since it's already in MooseObjectAction?
| const auto timeStop = std::chrono::high_resolution_clock::now(); | ||
| elapsedTime += std::chrono::duration<double, std::milli>(timeStop - timeStart).count() / 1e3; | ||
|
|
||
| _console << "OpenMC initialization took " << elapsedTime << " s" << std::endl; |
There was a problem hiding this comment.
Shouldn't we include OpenMCProblemBase::initialSetup()/OpenMCCellAverageProblem::initialSetup() in the timing? if this is irrelevant for this PR then maybe I'd suggest making this message clearer on what is being counted here as it might get confusing, since a significant amount of extra time can be spent during these other initialization steps
|
I realize you were using the mechanism in NekInitAction, but I noticed in MOOSE docs. here the part:
which suggests deriving from |
|
Maybe also consider adding params.addClassDescription() to OpenMCInitAction::validParams() for the generated docs |
3 participants
This PR adds a new action to initialize OpenMC.
Context
I am using Cardinal to transfer temperatures to OpenMC not to OpenMC cells, but to a custom temperature field using a regular mesh on the OpenMC side. The multiphysics coupling seems to work well with my local prototype so I am working on cleaning the different features I have to submit them as PRs here.
Description
I moved the initialization of OpenMC from the constructor of OpenMCProblemBase to its own action. The objective is to be able to initialize OpenMC once and potentially before the [Mesh] keyword is interpreted as I have a custom mesh generator (coming in a next PR) that requires having access to information from the XML file of OpenMC.
Open question
I mainly copied the NekInitAction mechanism. However, I am not sure if we need to protect this action against multiple calls as I do not know if Cardinal is able to call this action more than once. Please let me know if you think that I should add a protection against that.