openmc icon indicating copy to clipboard operation
openmc copied to clipboard

added material.from_library method

Open shimwell opened this issue 3 years ago • 17 comments

Hi all,

This PR attempts to add the ability to add material composition and density from a predefined collection / library.

This PR follows on from this discussion where adding materials from the PNNL library of predefined materials was discussed.

Adding a PNNL_v1 material database has previously be performed by PyNe and more recently by ATTILA. I've also done something similar before in the neutronics-material-maker package.

One of the differences to existing implementations and this PR is that we have revision version 2 (released in 2021) of the PNNL compendium released in here instead of the first revision (released in 2011)

I think this method for this implementation is quite concise due to expanding the dictionary into keyword arguments with ** . The implementation is also extendable as the JSON file can support multiple libraries if PNNL_v3 gets release or there is a need to add another material compendium.

I've also added a simple test to the test_material.py unit test but the usage is simple

>>>import openmc

>>>m1 = openmc.Material()
>>>m1.add_from_library(name='Sodium Oxide', library='pnnl_v2')
>>>m1
Material
	ID             =	1
	Name           =	
	Temperature    =	None
	Density        =	2.27 [g/cc]
	S(a,b) Tables  
	Nuclides       
	O16            =	0.332523     [ao]
	O17            =	0.0001266665 [ao]
	O18            =	0.0006833327 [ao]
	Na23           =	0.666667     [ao]

shimwell avatar Jul 05 '22 16:07 shimwell

@shimwell I really like this! My first bit of feedback is that I think having a single json file for each library may be clearer and I think the method should be a class method on the Material class so something more like this: m1 = openmc.Material.from_library(name='Sodium Oxide', library='pnnl_v2')

eepeterson avatar Jul 05 '22 18:07 eepeterson

Thanks for the feedback @eepeterson I've moved the method to a classmethod as suggested.

The python example from the first comment now changes to

>>>import openmc

>>>m1 = openmc.Material.from_library(name='Sodium Oxide', library='pnnl_v2')
>>>m1
Material
	ID             =	1
	Name           =	
	Temperature    =	None
	Density        =	2.27 [g/cc]
	S(a,b) Tables  
	Nuclides       
	O16            =	0.332523     [ao]
	O17            =	0.0001266665 [ao]
	O18            =	0.0006833327 [ao]
	Na23           =	0.666667     [ao]

shimwell avatar Jul 06 '22 10:07 shimwell

@eepeterson I've refactored the PR to use a single files for each library as suggested :+1: . I guess this will be handy when adding private material libraries.

shimwell avatar Jul 06 '22 11:07 shimwell

@shimwell Also looks like you have a conflict on material.py

paulromano avatar Jul 11 '22 17:07 paulromano

@paulromano and @shimwell I have been thinking about this a little lately and I think this might warrant just a little more structure to do robustly, namely a MaterialLibrary class with to/from file methods, the ability to handle elemental compositions in addition to nuclide compositions, as well as the ability to merge existing materials.xml files into a library, and nice printing/repr for QA purposes. I also think an optional environment variable like OPENMC_MATERIAL_LIBRARY_PATH as a colon separated list of paths (like $PATH) would be powerful and separate any internally stored libraries like the PNNL_v2 one in this PR from other custom libraries. All of these features are pretty straightforward, the only question in my mind is if we want the JSON format for libraries or if there is another preferred format. Thoughts?

eepeterson avatar Jul 11 '22 18:07 eepeterson

I was just looking at a nice PR https://github.com/openmc-dev/openmc/pull/2139 . Once #2139 is merged in then this PR could be refactored to make use of the new methods introduced. The JSON file could also be reduced in side considerably to dictionaries of tuples instead of the current rather verbose dictionary.

shimwell avatar Jul 28 '22 09:07 shimwell

@shimwell I think we need to move the library file to material_libraries/pnnl_v2.json to fix the failing test

eepeterson avatar Aug 05 '22 10:08 eepeterson

@shimwell I think we need to move the library file to material_libraries/pnnl_v2.json to fix the failing test

Thanks Ethan. I've created the folder, renamed the file and moved it in there. I've also added it to the package data in setup.py. Hoping the tests pass this time :eyes:

shimwell avatar Aug 05 '22 11:08 shimwell

Having a bit of a chat on slack and we are keen to change the underlying file format to XML.

We are also keen to keep the data file small (which requires elemental descriptions of the materials)

To proceed with this PR we need to add support for elements to the materials.XML file format and convert the JSON file on this PR to an XML file.

This PR will go on hold for a little time while we add element support to the materials.xml file

shimwell avatar Aug 08 '22 14:08 shimwell

I came across a little error that the CI didn't catch. Perhaps because we use pip install -e . which links but doesn't copy files

when using pip install . the pnnl_v2.xml file was not being copied over to the directory where python package was installed.

So I've added an __init__.py file to the openmc/data/material_libraries folder that helps the setup.py realize that these xml files should be copied

shimwell avatar Aug 10 '22 15:08 shimwell

I have left this PR for a while as the plan was to refactor the materials class so that is accepts elements then come back to this PR.

However I have an alternative suggestions.

How about we remove the openmc/data/material_libraries/pnnl_v2.xml file and the code that adds the internal library so we are no longer adding a specific PNNL library to the code. However we retain the functionality of adding a users xml material library file?

basically just retaining part of this PR so that users can load in their own material library.

This idea was inspired by @mwart-cfs who has a use case I had not considered. For QA purposes they prefer no material libraries to be initially loaded into the code but want the ability to add user libraries. Further details here

shimwell avatar Dec 08 '22 12:12 shimwell

I don’t quite see how not including widely used libraries within OpenMC directly helps with QA exactly and to me it doesn’t make sense for something like the PNNL compendium to exist within every users custom libraries. Since properly including elements in material specifications does require significant refactoring, maybe it is worth implementing a temporary way of including user libraries, but im not sure.

eepeterson avatar Dec 08 '22 13:12 eepeterson

How about we break this PR into two parts.

Part 1, just the frame work to allow users to load in their own custom libraries (xml files)

Part 2, after the material refactor add in the PNNL compendium so that it is distributed with openmc package and available as a default loaded in library

shimwell avatar Dec 08 '22 14:12 shimwell

I think this might be worth another go as the materials compendium is now downloadable in json format https://compendium.cwmd.pnnl.gov/

shimwell avatar Aug 23 '23 09:08 shimwell

Also there is now a Pyne materials package that creates pyne materials from the compendium. Nice work by @ahnaf-tahmid-chowdhury

https://github.com/pyne/materials-compendium

shimwell avatar Aug 23 '23 09:08 shimwell