Depcode openmc init #132

yardasol · 2022-02-16T21:29:14Z

Summary of changes

This PR is the first of a series that adds OpenMC support into SaltProc. The scope of this PR is running OpenMC and handiling input files. Specifically, this PR:

Removes non-critical parameters from the SaltProc input file:
- iter_inputfile
- iter_matfile
  - Relevant changes to the examples and test suite.
  - note: these parameters are now class attributes to avoid breaking the API
Changes the name of the template_inputfile_path input parameter to template_inputfiles_path.
- Relevant changes to the test suite, examples, and app.py
Adds depletion-code-dependent type mutability for the template_inputfiles_path through use of the allOf keywork in the input schema
Makes the following changes depcode.py:
- Proper documentation for the previously undocumentat param and sim_info attributes of Depcode and DepcodeSerpent
- iter_inputfile and iter_matfile are now class attributes instead of class parameters for all classes.
  - Relevant changes to the test suite and app.py
- A new class, DepcodeOpenMC
  - Fully implemented functions:
    - __init__()
    - run_depcode()
      - OpenMC's depletion functionality is all through the python API. Because of this, in order to use parallelism with MPI and OpenMP, this function runs a python script, openmc_deplete.py, that loads the input files and runs the depletion simulation through OpenMC.
    - switch_to_next_geometry()
    - write_depcode_input()
    - write_depletion_settings()
    - write_openmc_tallies()

Because this is such a large PR, we'll need to split up the review process:

@LukeSeifert (changes due to adjusting the input file JSON schema):
- saltproc/input_schema.json
- examples/
  - msbr/msbr_main.json
  - tap/tap_main.json
- saltproc/tests/
  - integration_tests/
  - const_repr/tap_main_test.json
  - no_repro/test_no_reproc_run.py
  - test.json
  - test_app.py
  - test_materialflow.py
  - test_process.py
  - test_separator.py
  - test_simulation.py
  - test_sparger.py
@gwenchee (additions to depcode.py)
- saltproc/
  - input_schema.json
  - depcode.py
  - openmc_deplete.py
  - app.py
@abachma2 (additions to depcode.py, alt)
- saltproc/
  - input_schema.json
  - depcode.py
  - test_depcode.py
@smpark7 (formatting and documentation changes)
- doc/releasenotes/v0.5.0.rst
- saltproc/simulation.py
- scripts/preprocessors/generate_mat_composition/README.md

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Required for Merging

Associated Issues and PRs

Issue: Partially addresses [Meta] Merge order for open PRs #151

Associated Developers

Dev: @

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

…strings

…file and iter_matfile as input parameters but maintain in the DepcodeSerpent class

…es in app.py

pep8speaks · 2022-02-17T14:43:58Z

Hello @yardasol! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-06-12 20:37:55 UTC

LukeSeifert

Reviewer 1 Files Reviewed

Overall looks good. There are some minor things I noticed, but I would say these files look clean. I'm not familiar with OpenMC, so if there are any typos/errors in those input files, I may have missed them.

saltproc/input_schema.json

saltproc/tests/test_data/openmc/test_materials.xml

saltproc/tests/test_depcode.py

saltproc/tests/test_sparger.py

yardasol · 2022-04-26T19:53:14Z

I'll be reviewer 2

@gwenchee bump

abachma2 · 2022-04-26T20:18:22Z

I'll review section 3.

abachma2

Review for Part 3

Overall this looks like some really good work @yardasol! I do have some questions about things that seem missing or inconsistent.

saltproc/depcode.py

abachma2 · 2022-04-26T20:25:43Z

saltproc/depcode.py

+    iter_inputfile : dict of str to str
+        Paths to OpenMC input files for OpenMC rerunning.
+    iter_matfile : str
+        Path to iterative, rewritable material file for OpenMC
+        rerunning. This file is modified during the simulation.


Are these still used? I looks like earlier on in the file all mentions of these attributes are removed from the code.

They are used. They used to be function parameters, but since they don't change within a simulation it makes sense to have them as attributes. Not sure where "earlier on in the file" refers to.

saltproc/depcode.py

abachma2 · 2022-04-26T20:28:22Z

saltproc/depcode.py

+    def read_depcode_info(self):
+        """Parses initial OpenMC simulation info from the OpenMC output files
+        and stores it in the `Depcode` object's ``sim_info`` attribute.
+        """
+
+    def read_depcode_step_param(self):
+        """Parses data from OpenMC depletion output for each step and stores
+        it in `Depcode` object's ``param`` attributes.
+        """


Are these functions supposed to do anything? I don't see any text for them to do anything.

They are uninplemented right now. They must be at least defined because they are abstract funtions in the Depcode abstract class.

abachma2 · 2022-04-26T20:28:52Z

saltproc/depcode.py

+    def read_dep_comp(self, read_at_end=False):
+        """Reads the depleted material data from the OpenMC depletion


Same comment as above.

see response above

abachma2 · 2022-04-26T20:30:15Z

saltproc/depcode.py

+    def write_mat_file(self, dep_dict, dep_end_time):
+        """Writes the iteration input file containing the burnable materials


Same comment as above.

see response above

saltproc/depcode.py

saltproc/tests/test_depcode.py

gwenchee

Hey @yardasol, overall looks good. I have some comments about some software decisions and documentation. Sorry for the delay

saltproc/depcode.py

gwenchee · 2022-04-27T23:03:33Z

saltproc/depcode.py

@@ -16,14 +17,24 @@ class Depcode(ABC):
    population, active, and inactive cycles. Contains methods to read template
    and output files, and write new input files for the depletion code.

+    Attributes
+    -----------
+    param : dict of str to type


It's not clear what this str or type is.

I've expanded the docstring.

gwenchee · 2022-04-27T23:04:56Z

saltproc/depcode.py

-           iter_matfile : str
-               Name of iterative, rewritable material file for depletion code
-               rerunning. This file is modified during  the simulation.
+           template_inputfiles_path: str or dict of str to str


This description is also not clear. What str?

template_inputfiles_path is a string or a dict where the key is a string and the value is a string.

I meant that it's not clear what the str is or dict. If str, is it the solver name?

I've expanded the docstring.

gwenchee · 2022-04-27T23:07:38Z

saltproc/depcode.py

+
+    Attributes:
+    -----------
+    param : dict of str to type


Again, not clear. What str and what type? Could provide more info in the next line

I expanded the docstring.

saltproc/depcode.py

gwenchee · 2022-04-27T23:20:44Z

saltproc/depcode.py

 class DepcodeSerpent(Depcode):
    r"""Class contains information about input, output, geometry, and
    template files for running Serpent2 depletion simulations.
    Also contains neutrons population, active, and inactive cycles.
    Contains methods to read template and output files,
    write new input files for Serpent2.

+    Attributes
+    -----------
+    param : dict of str to type


Same comment. Not clear what str and type are

I expanded the docstring.

gwenchee · 2022-04-27T23:20:56Z

saltproc/depcode.py

+    -----------
+    param : dict of str to type
+        Holds Serpent depletion step parameter information
+    sim_info : dict of str to type


Same comment. Not clear what str and type are

I expanded the docstring.

gwenchee · 2022-04-27T23:21:22Z

saltproc/depcode.py

-                 template_inputfile_path="reactor.serpent",
-                 iter_inputfile="data/saltproc_reactor",
-                 iter_matfile="data/saltproc_mat",
+                 template_inputfiles_path="reactor.serpent",


I second what Amanda said about adding an _ between input and files

okay it has been done

saltproc/openmc_deplete.py

gwenchee · 2022-04-27T23:23:52Z

saltproc/openmc_deplete.py

+import argparse
+
+
+def parse_arguments():


Why are you allowing the XML files to be in different locations? With normal OpenMC runs, users are expected to have XML files in the same path no?

This is just for flexibility. In practice, the user doesn't call this function at all. See the DepcodeOpenMC.run_depcode function for an example.

It's also an artifact of how I've defined the template_inputfiles_path variable, which has a path for each input file. Normally OpenMC expects the material, geometry, and settings files to be names materials.xml, geometry.xml, and settings.xml respectively. In order for geometry changing to work, we need to be able to have multiple different geomery files in one directory, so they can't all be names geometry.xml. Similar, we have an initial material composition that we start the simulation with, but we also have an iteration material file for each depletion step. If both of these had the name materials.xml, they could overwrite each other.

smpark7

Review for part 4.

Looks mostly good and clean. I added some comments for typos and for clarification on the non-critical input parameters that you're removing.

doc/releasenotes/v0.5.0.rst

's comments

…t_file_path

yardasol · 2022-06-12T22:24:26Z

@LukeSeifert @gwenchee @smpark7 @abachma2 now that the CI is working, is this good for merge?

yardasol · 2022-06-13T14:52:41Z

Actually this should probably be rebased since it's been open for months

yardasol · 2022-06-29T02:43:55Z

@smpark7 I think I made the requested changes

yardasol added 24 commits February 16, 2022 10:31

update version to 0.5.dev-0

2a91417

created v0.5.0 release notes

55502dc

add flow control for creatind DepcodeOpenMC object in app.py

a9654f1

Added DepcodeOpenMC class

1c351b5

add import statements to app.py and depcode.py

8fe45ad

add function signatrues to DepcodeOpenMC; update docstrings

3ddb498

generalize Depcode function docstrings

d99f538

update DepcodeOpenMC function docstrings

bce656b

remove serpent-specific attributes from the Depcode class; update doc…

dec07e6

…strings

start writing OpenMC.run_depcode()

6d015ff

add option for openmc input files to the sceha

8651a5c

pass input schema changes to app.py and depcode.py; remove iter_input…

ae5b8c1

…file and iter_matfile as input parameters but maintain in the DepcodeSerpent class

consistency changes for unit tests

e6a27dc

change default values for depcode.py

e33d080

update the v0.5.0 releasenotes

05049c7

propogate input file changes to the integration tests

684956d

inital version of DepcodeOpenMC.write_depcode_input

68c6c67

start write_depletion_settings; implement switch_to_next_geometry

6f823d6

DepcodeOpenMC function advancements

d3b73a0

further advancement and refinement of DepcodeOpenMC functions; bugfix…

1140db3

…es in app.py

Added test_openmc.json saltproc input file to unit tests

5db3c77

added python script for running openmc depletion in parallel

122a282

consistency changes between app.py and depcode.py

935125f

start updating v0.5.0 releaes notes

18963e4

yardasol added 5 commits February 17, 2022 12:05

outline script to convert serpent materials to openmc materials

0151eeb

fill out most of the remaining functionality in serpent_to_openmc.py

64ff127

change name of material conversion script

1375cb0

typo fix

d970007

typo fix

a18d524

LukeSeifert reviewed Apr 20, 2022

View reviewed changes

yardasol requested review from abachma2 and ZoeRichter April 26, 2022 20:14

abachma2 reviewed Apr 26, 2022

View reviewed changes

gwenchee approved these changes Apr 27, 2022

View reviewed changes

smpark7 requested changes May 2, 2022

View reviewed changes

doc/releasenotes/v0.5.0.rst Outdated Show resolved Hide resolved

doc/releasenotes/v0.5.0.rst Outdated Show resolved Hide resolved

doc/releasenotes/v0.5.0.rst Outdated Show resolved Hide resolved

yardasol added 7 commits May 3, 2022 14:01

address @abachma2

10615ba

's comments

typo fixes and docstring expansion (via @gwenchee's review comments)

c154b21

@smpark7's suggestions

c64dc81

minor typo fix in description for template_inputfiles_path

83378ad

remove raw string tag in depcode class docstrings

4dddc52

add TODO tag to comment; remove cruft comment in depcode.py

b4ec66d

change input file variable: template_inputfiles_path -> template_inpu…

7e128a2

…t_file_path

yardasol requested review from gwenchee, smpark7, LukeSeifert and abachma2 May 3, 2022 20:05

yardasol and others added 3 commits May 3, 2022 15:06

expand docstring for template_input_file_path in Depcode class

0328fbb

expand description for template_input_file_path in DepcodeOpenMC

6caa1ad

Contribution -> Contributions

b867323

smpark7 approved these changes Jun 29, 2022

View reviewed changes

smpark7 merged commit 25b85c4 into arfc:master Jun 29, 2022

yardasol removed the Status:4-In Progress label Mar 7, 2023

		def read_dep_comp(self, read_at_end=False):
		"""Reads the depleted material data from the OpenMC depletion

		def write_mat_file(self, dep_dict, dep_end_time):
		"""Writes the iteration input file containing the burnable materials

Depcode openmc init #132

Depcode openmc init #132

Conversation

yardasol commented Feb 16, 2022 • edited Loading

Summary of changes

Types of changes

Required for Merging

Associated Issues and PRs

Associated Developers

Checklist for Reviewers

pep8speaks commented Feb 17, 2022 • edited Loading

Comment last updated at 2022-06-12 20:37:55 UTC

LukeSeifert left a comment

Choose a reason for hiding this comment

Reviewer 1 Files Reviewed

yardasol commented Apr 26, 2022

abachma2 commented Apr 26, 2022

abachma2 left a comment

Choose a reason for hiding this comment

Review for Part 3

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwenchee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gwenchee Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smpark7 left a comment

Choose a reason for hiding this comment

yardasol commented Jun 12, 2022

yardasol commented Jun 13, 2022

yardasol commented Jun 29, 2022

yardasol commented Feb 16, 2022 •

edited

Loading

pep8speaks commented Feb 17, 2022 •

edited

Loading

gwenchee Apr 29, 2022 •

edited

Loading