Add call to the OpenMC Depletion Solver #13

abachma2 · 2023-04-25T20:29:57Z

This PR adds new functionality to the DepleteReactor archetype:

Call the OpenMC depletion sovler during the transmute method
Write the resulting composition to an xml file
Add state variables for the path to the directory with OpenMC model files and the name of the decay chain file

The CI fails because importing openmc into the archetype (i.e. import openmc) causes a segmentation fault. The seg fault is after the simulation runs and writes the results to the output file and is trying to close python imports (I think). So it all still mostly runs, and I have manually compared the results of the output database to the expected values in the test suite. An issue (#12) has been created to help resolve the seg fault.

Add state variable definition and default. Add mention to phase 1 of DRE, but not sure it is any meaningful functionality. Tests all pass

New functions to get the index of a materials commodity name, recipe name, or preference based on the material's object ID. These functions exist in cycamore:Reactor, but were previously overlooked

add an if loop to only get the material if its commodity name is in the spent materials dictionary. Call the get_commods function instead of just looping of the commodity names list. 3 tests failing, two of which concern the resources

The loop for the recipes was creating a bid for each possiblespent fuel recipe, which is unphysical. Each spent fuel assembly will only have one composition. Instead, get the index of the commodity name in fuel_outcommods and matches to the recipe at the same index of fuel_outrecipes. Enforces the need for users to have consistent ordering in their input file

Had to manually call each step of the Depletion.run_depletion because it just would not work if I tried to call that method directly

smpark7 · 2023-05-30T18:46:08Z

.github/workflows/test-openmcyclus.yml

      - name: Setup Conda
        uses: conda-incubator/setup-miniconda@v2
        with:
-          miniforge-variant: Mambaforge
          miniforge-version: latest
          activate-environment: openmcyclus-env
-          environment-file: environment.yml
-          use-mamba: true  

      - name: Conda config
        run: |
          conda config --env --set pip_interop_enabled True 
-
-      - name: Update Environment
-        run: |
-          mamba env update -n openmcyclus-env -f environment.yml

      - name: Install pytest
        run: conda install -y pytest

      - name: Build OpenMC 
        run: |
-          mamba install openmc
+          conda install -y python=3.7 cyclus cycamore 
+          HDF5_DIR=$CONDA_PREFIX && pip install h5py
+
+      - name: OpenMC dependencies
+        run: |
+          sudo apt -y update
+          sudo apt install -y libhdf5-dev
+
+      - name: Download OpenMC
+        uses: actions/checkout@v3
+        with: 
+          repository: openmc-dev/openmc
+          path: openmc
+          submodules: recursive
+
+      - name: Build OpenMC from source
+        run: |
+          run: build-openmc.sh
+

      - name: Install OpenMC cross section library
        run: echo "OPENMC_CROSS_SECTIONS=cross_sections.xml" >> $GITHUB_ENV


Have you tested this workflow yet? If not, you could merge into your fork's main branch to test it

It looks like right now there is a syntax error in the ci scripts, so I will fix that. But because of a seg fault when importing OpenMC into a cyclus script (See issue #12), there isn't much point in running the integration tests in a CI environment, because they will all fail due to the seg fault. I can make sure that the environment builds and the unit tests pass, but that's the best I can do until Issue #12 is resolved.

Oh yes sorry for not reading your PR comments properly

All good. It's important to have the CI as functional as possible. I'll fix the syntax error int he CI on this branch, but do the bulk of the CI changes/maintenance on another branch to keep this PR from getting too big.

nsryan2

Mostly LGTM, I saw Sun Myung's comment so I won't approve quite yet, I had some minor README comments, package comments, and some stuff on the tests.

nsryan2 · 2023-05-30T18:29:01Z

.github/workflows/test-openmcyclus.yml

-          pytest --ignore tests/unit_tests
+          pytest 
+
+FWAk8hnxPUNDeep!


What does this mean?

I'm not sure how that ended up there. I must have accidentally copied whatever was in my clipboard into the file. It's been removed!

README.md

nsryan2 · 2023-05-30T18:32:22Z

README.md

+### Install OpenMCyclus
+To install this archetype library run ``pip install .``. 
+To run tests: ``pytest`` from the main directory.
+


Suggested change

### Testing

In progress.

Should there be a testing subsubsection?

All it would contain would be the sentence about running pytest.

README.md

openmcyclus/DepleteReactor.py

nsryan2 · 2023-05-30T18:49:47Z

tests/integration_tests/test_depletereactor.py

-        assert all(quantities == [10]*6)
+        assert len(tbl) == 9
+        assert all(times == [0,0,0,2,2,5,5,8,8])
+        assert all(quantities == [10]*9)


Suggested change

assert all(quantities == [10]*9)

assert all(quantities == [10]*len(tbl))

Would it reduce the chance for errors if you add times later if you changed it to this? For test_resources too?

This part of the code is testing what's in the output database. So your code suggestion is just testing if all of the entries in the table are of value 10, not specifically if there are 9 occurrences of value 10 in the table. I suppose that by testing for [10]*9 that's also checking that there are 9 values in the table, same as l.124. but I'm okay with being redundant in my testing.

smpark7

I didn't run anything but I looked through your code. Here are my review comments.

.github/workflows/test-openmcyclus.yml

README.md

smpark7 · 2023-05-30T18:52:14Z

examples/complex.xml

+            <model_path>/home/abachmann/openmcyclus/tests/</model_path>
+            <chain_file>chain_endfb71_pwr.xml</chain_file>


Can this be a relative path? It won't work on other people's computers.

Maybe? I'll play around with it. Can we make this an issue so that this doesn't hold up the merging of this PR?

Sure that's okay

examples/simple.xml

openmcyclus/DepleteReactor.py

smpark7 · 2023-05-30T18:59:35Z

openmcyclus/DepleteReactor.py

+        for commod_index, commod in enumerate(self.fuel_outcommods):
            reqs = requests[commod]
+            print("time:", self.context.time, "reqs:", reqs)
            if len(reqs) == 0:
                continue
            elif (got_mats == False):
                all_mats = self.peek_spent()
-
            if len(all_mats) == 0:
                tot_qty = 0
                continue
-            mats = [all_mats[commod]]
+            if commod in all_mats: 
+                mats = [all_mats[commod]]
+
+            else:
+                mats = []
+            print("time:", self.context.time, "mats to trade matching request commod:", mats)
            if len(mats) == 0:
-                continue
-            for ii in range(len(self.fuel_outrecipes)):
-                recipe_comp = self.context.get_recipe(self.fuel_outrecipes[ii])
-                for req in reqs:
-                    tot_bid = 0
-                    for jj in range(len(mats)):
-                        tot_bid += mats[jj].quantity
-                        qty = min(req.target.quantity, self.spent_fuel.quantity)
-                        mat = ts.Material.create_untracked(qty, recipe_comp)
+                continue 
+
+            recipe_comp = self.context.get_recipe(self.fuel_outrecipes[commod_index])
+
+            for req in reqs:
+                tot_bid = 0
+                for jj in range(len(mats)):
+                    tot_bid += mats[jj].quantity
+                    qty = min(req.target.quantity, self.assem_size)
+                    mat = ts.Material.create_untracked(qty, recipe_comp)
+                    for kk in range(self.spent_fuel.count):


Nice refactoring. You may wanna update your for-loop index names.

In what way? How would you suggest updating the index names?

Renaming jj and kk as ii and jj. But leaving it as is is fine too

openmcyclus/DepleteReactor.py

smpark7

LGTM! @nsryan2 go ahead and merge if you approve too.

abachma2 added 29 commits February 22, 2023 13:22

add notes about installing deps

aca273b

update install notes

7e869f7

call Depletion class in archetype

5b65d6a

add some install directions

dca62de

update install for ci environment and README

2a35f63

remove duplicate steps, remove some conda dependencies

14ac0b3

remove step with mamba

0f39126

slim down conda dependencies

ea51253

fix spelling error

89d7ecb

add full list of conda installs back in

bdc1475

reduce conda install to cyclus and python

5b196ef

attempt to configure update build env, not working

f863818

start adding fuel preferences

16ee3f6

Add state variable definition and default. Add mention to phase 1 of DRE, but not sure it is any meaningful functionality. Tests all pass

update test for complex case to include material preferences

bd06b3e

update input description

7b6945e

update TestSimple to match cycamore results

cdfb41e

add print statements to see what's going on

8f17f46

add print statements to see what's going on

9491fee

add new functions

28c6ed8

New functions to get the index of a materials commodity name, recipe name, or preference based on the material's object ID. These functions exist in cycamore:Reactor, but were previously overlooked

adjust discharge function

f78f6d6

add an if loop to only get the material if its commodity name is in the spent materials dictionary. Call the get_commods function instead of just looping of the commodity names list. 3 tests failing, two of which concern the resources

change recipe for mox to be distinguished from uox

60a7ad7

change testing for uox and spent uox not in transactions table

3a599ff

fix not trading all remaining spent fuel for decommissioning

86ff272

Merge branch 'preferences' into depletion

1794b3e

Merge branch 'main' into depletion

894cf26

add calls to Depletion methods in archetype

b9be155

call openmc depletion solver and create recipe

1d819ed

Had to manually call each step of the Depletion.run_depletion because it just would not work if I tried to call that method directly

add path state variables, remove hardcoded paths

29b0b88

abachma2 requested a review from yardasol April 25, 2023 20:30

abachma2 requested review from smpark7, LukeSeifert, samgdotson and nsryan2 April 25, 2023 20:30

abachma2 self-assigned this Apr 25, 2023

Merge branch 'main' into depletion

d51a5e6

smpark7 reviewed May 30, 2023

View reviewed changes

nsryan2 reviewed May 30, 2023

View reviewed changes

smpark7 requested changes May 30, 2023

View reviewed changes

abachma2 added 6 commits May 30, 2023 14:19

remove extrenuous text at the bottom

78be065

update dependency install isntructions

54de47e

fix tag closes in input structure

df94f37

fix indention issue

a1755d3

remove unused package imports

97d3020

fix / location in tag closing

2ae41a9

abachma2 requested review from nsryan2 and smpark7 May 30, 2023 20:40

smpark7 approved these changes May 30, 2023

View reviewed changes

nsryan2 merged commit 8338002 into arfc:main May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add call to the OpenMC Depletion Solver #13

Add call to the OpenMC Depletion Solver #13

abachma2 commented Apr 25, 2023

smpark7 May 30, 2023

abachma2 May 30, 2023

smpark7 May 30, 2023

abachma2 May 30, 2023

nsryan2 left a comment

nsryan2 May 30, 2023

abachma2 May 30, 2023

nsryan2 May 30, 2023

abachma2 May 30, 2023

nsryan2 May 30, 2023

abachma2 May 30, 2023

smpark7 left a comment

smpark7 May 30, 2023

abachma2 May 30, 2023

smpark7 May 30, 2023

smpark7 May 30, 2023

abachma2 May 30, 2023

smpark7 May 30, 2023

smpark7 left a comment

	assert all(quantities == [10]*9)
	assert all(quantities == [10]*len(tbl))

		<model_path>/home/abachmann/openmcyclus/tests/</model_path>
		<chain_file>chain_endfb71_pwr.xml</chain_file>

Add call to the OpenMC Depletion Solver #13

Add call to the OpenMC Depletion Solver #13

Conversation

abachma2 commented Apr 25, 2023

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

nsryan2 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

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

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