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

Use subchannel in OpenmcHeatDriver #59

Merged
merged 8 commits into from
Aug 12, 2019

Conversation

aprilnovak
Copy link
Contributor

This MR adds use of the subchannel fluid temperature and density solutions in OpenMC for the OpenmcHeatDriver. Auxiliary features include:

  1. Ability to set initial density based on OpencMC model
  2. Ability to set initial fluid temperature based on OpenMC model

This MR doesn't include any checks on convergence based on density or fluid temperature, which I think should be added as a separate commit. In the design here, I found it easier to work with temperatures and densities for the fluid and solid separately so that not a whole lot of reworking was needed for the existing solid solution. This results in the fluid_mask being un-used. I think this is fine for now, but I envision in the future moving fluid_mask out of the T/H base class and into NekDriver, or changing the internals of SurrogateHeatDriver to make it necessary to use.

This is marked as WIP until !58 is merged because I found the additional cell fissionable checks to be useful in debugging.

src/openmc_driver.cpp Outdated Show resolved Hide resolved
src/openmc_driver.cpp Outdated Show resolved Hide resolved
src/openmc_driver.cpp Outdated Show resolved Hide resolved
@RonRahaman
Copy link
Collaborator

@aprilnovak The issues with variable sizes are spread throughout a lot of the project, and I just commented on the parts that you touched in this PR. Some of these might be better addressed elsewhere in the code you didn't touch (for example, I can see some of them being addressed in class declarations that you didn't touch here). If you feel that way, please feel free to ignore my comments on a case-by-case basis.

@aprilnovak
Copy link
Contributor Author

Thanks for the detailed review @RonRahaman! I haven't been consistent with using gsl::index, thanks for reminding me of that. I think this could be address in a separate MR that uses it throughout the project so that we're consistent everywhere. I'll put in a separate issue for that

Copy link
Member

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this! Look forward to using it.

include/enrico/openmc_heat_driver.h Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
src/openmc_heat_driver.cpp Outdated Show resolved Hide resolved
@aprilnovak
Copy link
Contributor Author

Thanks for the detailed review @RonRahaman! I've decided to address the narrowing issues here, I'll go after other ones in the project for #61.

@aprilnovak aprilnovak changed the title WIP: Use subchannel in OpenmcHeatDriver Use subchannel in OpenmcHeatDriver Aug 9, 2019
@paulromano
Copy link
Member

There are some bugs with how the indices are being used to access z_ and solid_areas_ (that's why CI is failing). I'll send you a patch that fixes the bugs and does some renaming as well. I find it clearer to use the names solid_index and fluid_index.

@paulromano paulromano merged commit 436f39d into enrico-dev:master Aug 12, 2019
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.

3 participants