Update to use OpenMC v0.14.0, update documentation #19

abachma2 · 2024-04-23T16:10:31Z

This PR does some updates to the archetype library to be compatible with OpenMC v0.14.0. A full list of changes in this PR (it suffers from scope creep unfortunately):

Updating DepleteReactor archetype to now accept thermal power (instead of just using the power_cap parameter for depletion) and flux (required input for OpenMC IndependentOperator
Remove the procedure to check for previous core composition -- this was a quick implementation that should really be thought through more for better implementation.
Update methods in Depletion class to be compatible with OpenMC v0.14.0, and associated tests
Running integration tests only on PR, and not actually running the tests (we know they fail because of issue Importing OpenMC causes a seg fault #12)
Add unit tests for the openmcyclus.Depletion class to CI (with help from @ceserz2)
Add .gitignore for repo
Improve documentation, which includes updating the README (change format from md to rst), adding a CHANGELOG, and CONTRIBUTING guidelines
Updating examples/micro_xs.csv to format used in OpenMC v0.14.0
Updating inputs in examples directory to include the new parameters added to DepleteReactor
Add comparison/materials.xml and comparison/chain_endfb71_pwr.xml for better reproduction of running the benchmark directory
Update the comparison/OpenMC.ipynb for OpenMC v0.14.0

commented out to prevent running test that is known to fail

yardasol

Hi @abachma2,

I can tell you put a lot of work into this. Nice job. My comments are below.

yardasol · 2024-04-24T20:02:41Z

.github/workflows/unit-test.yml

Consider bundling these changes into the existing test-openmcyclus workflow.

Are you referring to the build process? Or the tests themselves? I was planning on streamlining the build process in a different PR so this one didn't get too big.

I do plan to have another PR later on for checking if the CHANGELOG is updated. I'm not sure if that's what you were referring to here.

CHANGELOG.rst

CONTRIBUTING.rst

yardasol · 2024-04-24T20:19:12Z

README.rst

+    ~ $ conda install -y gxx_linux-64 gcc_linux-64 cmake make git glib libxml2 libxmlpp-4.0 liblapack pkg-config coincbc boost-cpp hdf5 sqlite pcre setuptools pytest pytables pandas jinja2 cython websockets pprintpp pip mamba
+
+    ~ $ mamba install -y openmc=0.14.0 scipy=1.11
+
+    ~ $ git clone https://github.com/cyclus/cyclus.git
+
+    ~ $ cd cyclus
+
+    ~/cyclus $ python install.py


If people try to run these commands there will be errors. I get what you're going for, trying to emulate what the terminal would look like. Just something to consider.

What do you mean? These are the commands I put into my terminal. Is it because of the ~ $ formatting? I'm trying to show some of the directory structure for being inside the cyclus directory to install it. Do you have any suggestions for how it might be clearer?

Yes, I am referring to the ~ $ formatting. If your intention is more to show how these commands would be in a terminal rather than have users copy and paste them, then this is fine.

yardasol · 2024-04-25T15:39:32Z

openmcyclus/depletion.py

@@ -165,16 +162,15 @@ def get_spent_comps(self, material_ids, path):
            list of the compositions from the OpenMC model
        '''
        results = od.Results(path + "depletion_results.h5")
+        nuclides = od.MicroXS.from_csv(path+"micro_xs.csv").nuclides


Why load in the MicroXS object again? Why not just use self.micro_xs.nuclides?

Because I don't actually initialize that object with the class. That might be a cleaner way to do this though. I don't have strong feelings either way. I think initially I was thinking that you wouldn't always need that, but it also might be good to initialize the materials with the class as well.

I'm giving this a shot (initializing the cross sections and materials with the class) see the changes in commit c1d7d2 and let me know what you think.

openmcyclus/depletion.py

yardasol · 2024-04-25T15:42:36Z

tests/unit_tests/test_depletion.py

+        assert spent_comps[0][922350000] == 10.650004036820036
+        assert spent_comps[0][942390000] == 0.22663550016678385


Might be more robust to have a reference_depletion_results.h5 that you load in and compare against the spent comps. Not sure if it's that simple with Cyclus.

That's probably a cleaner way to do, and more regression style.

openmcyclus/DepleteReactor.py

Initializing these objects with the class removes extra calls to the files, and removes the need for the methods to create the objects. Still need to test out with Cyclus integration, but unit tests pass

yardasol

Hi @abachma2,
Good work on these changes. My other comments are below. I think this is getting pretty close, so I'm going to tentatively approve.

openmcyclus/depletion.py

yardasol · 2024-05-06T18:52:05Z

openmcyclus/depletion.py

-    def read_materials(self, path):
-        '''
-        Read in the materials file for OpenMC
-
-        Parameters:
-        -----------
-        path: str
-            path to the materials.xml file
+        self.microxs = od.MicroXS.from_csv(str(path + "micro_xs.csv"))
+        self.materials = openmc.Materials().from_xml(str(path + "/materials.xml"))

-        Returns:
-        ---------
-        materials: openmc.Material object
-            Materials for OpenMC
-        '''
-        materials = openmc.Materials()
-        materials = materials.from_xml(str(path + "/materials.xml"))
-        return materials


Good change. This was a lot for just two lines of code.

yardasol · 2024-05-06T18:54:49Z

openmcyclus/depletion.py

+        self.microxs = od.MicroXS.from_csv(str(path + "micro_xs.csv"))
+        self.materials = openmc.Materials().from_xml(str(path + "/materials.xml"))


These objects are already initialized in the enter_notify method of the DepleteReactor, and initializing them in the Depletion class doesn't work when running the archetype. This method requires more Depletion class method inputs, but it all runs

abachma2 · 2024-05-06T20:59:00Z

So it turns out that initializing the micro_xs and materials objects in the Depletion class doesn't work so well when running this in Cyclus (I could have sworn this was working a week or two ago). So I updated things to initialize these objects with the DepleteReactor class, and use them as inputs to methods in the Depletion class. The model_path attribute of the DepleteReactor class is read in as an input to the Depletion class to prevent having to re-input that information. The tests should be updated to incorporate these changes.

THe new version of these codes (v1.6) is now live in conda, and it'll be easier to install them using this method.

abachma2 · 2024-05-24T13:54:13Z

Bumping this for review if anyone is available right now.

yardasol

Hi @abachma2. Good job as always. I have a few more suggestions for you. Apologies for the long wait on this.

openmcyclus/DepleteReactor.py

yardasol · 2024-05-29T14:18:14Z

openmcyclus/DepleteReactor.py

+        # ss = str(len(assemblies)) + " assemblies"
        # self.record("TRANSMUTE", ss)


Consider removing these if they are not going to be used.

Any instances of the record method are intended to be there, but I need to dig into the cyclus python API for writing to the database some to make sure it all works as it should/does in the c++ api

Can you explain the purpose of having this method present in the code but commented out? Is there still something you are debugging? Or does it need to be there commented out to work properly?

The self.record method writes information to the database, and in this case trying to record when different reactor-related events occur. When I was working on this archetype, I noticed that it this method wasn't working as expected, likely because of how the Cyclus python API implements it. So I commented it out to prevent it from hindering other development work on the archetype. I didn't remove it, because I do want this functionality in the code, but it's not crucial. I will make an issue about getting this working to make sure it doesn't get forgotten.

openmcyclus/depletion.py

README.rst

openmcyclus/depletion.py

tests/unit_tests/test_depletion.py

abachma2 · 2024-05-30T14:40:32Z

Thanks for the feedback @yardasol! It should all be addressed now.

yardasol

Hi @abachma2,
Good work here. Just a couple more fixes and we should be g2g.

openmcyclus/DepleteReactor.py

yardasol · 2024-06-05T18:04:27Z

openmcyclus/DepleteReactor.py

+        # ss = str(len(assemblies)) + " assemblies"
        # self.record("TRANSMUTE", ss)


Can you explain the purpose of having this method present in the code but commented out? Is there still something you are debugging? Or does it need to be there commented out to work properly?

openmcyclus/depletion.py

tests/unit_tests/test_depletion.py

yardasol

Great job @abachma2! Sorry this PR took so long to review.

abachma2 · 2024-06-13T17:45:16Z

Thanks for the approval @yardasol. Are we waiting on something else for this to be merged?

abachma2 and others added 30 commits June 19, 2023 15:51

update tests and streamline depletion functions

94ee041

update reading materials function from Depletion class

3f3ebc9

add depletion results for testing suite

2def7fe

update comp to h5 committed to repo

c27af33

add unit test file

ef029ce

comment out integration test

711cc54

commented out to prevent running test that is known to fail

adding the rest of the steps to build the environment

ff0d7a4

adding "run tests" step

8f0f272

adding "install numpy" step

ed82f4d

adding "install mamba from conda" step

23ac454

adding a new install command for mamba

2a96a4b

updating conda install for mamba

71f9ec1

adding "checking environment step"

23847b4

commenting out "install mamba from conda" step

158246b

adding "conda info" step

075eb92

adding "conda info" to steps

deb9107

fixing "conda info" steps

8ad3391

updating environment setup

b402679

fixing more errors with environment activation

e79b3c7

removing environment activation step

e278951

fixing errors in setting up environment

fe212d8

updating environment command added

fa131ec

fixing "activate environment" function for each step

a551733

more fixes on code

8e28507

inserting "conda init" command to initialize shell

2b715e2

fixing command to activate the environment

3feec33

trying a new method for environment activation

7b7cae9

trying to update code to fix CI file

f283b3e

Merge branch 'ci' of github.com:ceserz2/openmcyclus_cz into tests

e7a8ccf

add gitignore file

53dc24c

abachma2 added the Type:Test Is related to software testing. Failing tests, necessary new tests, test frameworks, etc. label Apr 23, 2024

abachma2 added 4 commits April 23, 2024 11:18

pep8 fixes

0fa32d9

add new params, update install instructions

6b432e7

correct code block format

02b8637

update deps install and code format for input

3bd9f4c

yardasol reviewed Apr 25, 2024

View reviewed changes

abachma2 added 2 commits April 25, 2024 11:03

make some of the suggestions from @yardasol

9eae286

initialize microxs and materials with class

c1d7d27

Initializing these objects with the class removes extra calls to the files, and removes the need for the methods to create the objects. Still need to test out with Cyclus integration, but unit tests pass

abachma2 mentioned this pull request May 2, 2024

Accept multiple fluxes #20

Open

abachma2 requested a review from yardasol May 6, 2024 15:43

yardasol approved these changes May 6, 2024

View reviewed changes

abachma2 added 3 commits May 6, 2024 15:48

pep8 fixes, update documentation

de80f4b

pep8 fixes

c98667b

abachma2 requested a review from yardasol May 6, 2024 20:59

change cyclus/cycamore install instructions

f07eb28

THe new version of these codes (v1.6) is now live in conda, and it'll be easier to install them using this method.

yardasol requested changes May 29, 2024

View reviewed changes

abachma2 added 2 commits May 30, 2024 09:25

address comments from @yardasol

96eb863

fix test suite, function inputs and locations

1d1568a

abachma2 requested a review from yardasol May 30, 2024 14:40

update OpenMC footnote

3aecd12

yardasol requested changes Jun 5, 2024

View reviewed changes

address docstring comments from @yardasol

34ab439

abachma2 requested a review from yardasol June 5, 2024 18:41

yardasol approved these changes Jun 13, 2024

View reviewed changes

yardasol merged commit 1ad3ae1 into arfc:main Jun 13, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to use OpenMC v0.14.0, update documentation #19

Update to use OpenMC v0.14.0, update documentation #19

abachma2 commented Apr 23, 2024 •

edited

Loading

yardasol left a comment

yardasol Apr 24, 2024

abachma2 May 2, 2024

abachma2 May 6, 2024

yardasol Apr 24, 2024

abachma2 Apr 25, 2024

yardasol May 6, 2024

yardasol Apr 25, 2024

abachma2 Apr 25, 2024

abachma2 Apr 25, 2024

yardasol Apr 25, 2024

abachma2 Apr 25, 2024

yardasol left a comment

yardasol May 6, 2024

yardasol May 6, 2024

abachma2 commented May 6, 2024

abachma2 commented May 24, 2024

yardasol left a comment

yardasol May 29, 2024

abachma2 May 30, 2024

yardasol Jun 5, 2024

abachma2 Jun 5, 2024

abachma2 commented May 30, 2024

yardasol left a comment

yardasol Jun 5, 2024

yardasol left a comment

abachma2 commented Jun 13, 2024

		assert spent_comps[0][922350000] == 10.650004036820036
		assert spent_comps[0][942390000] == 0.22663550016678385

		self.microxs = od.MicroXS.from_csv(str(path + "micro_xs.csv"))
		self.materials = openmc.Materials().from_xml(str(path + "/materials.xml"))

		# ss = str(len(assemblies)) + " assemblies"
		# self.record("TRANSMUTE", ss)

Update to use OpenMC v0.14.0, update documentation #19

Update to use OpenMC v0.14.0, update documentation #19

Conversation

abachma2 commented Apr 23, 2024 • edited Loading

yardasol 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

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

yardasol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abachma2 commented May 6, 2024

abachma2 commented May 24, 2024

yardasol 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

abachma2 commented May 30, 2024

yardasol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yardasol left a comment

Choose a reason for hiding this comment

abachma2 commented Jun 13, 2024

abachma2 commented Apr 23, 2024 •

edited

Loading