added reference to material class and xml output #1817
Conversation
|
I think this is definitely worth it @shimwell. Thanks for the addition! This would help with QA for sure and would pair well with a future materials database PR. |
|
@shimwell looking at your example, it seems like |
|
I make use of Material().name in DAGMC models as the name corresponds to the h5m tag within the model However DAGMC tags are limited to 28 characters So I would typically like something like this that supports longer strings for additional details my_mat=openmc.Material(name='magnet_shielding')
my_mat.temperature=500
my_mat.reference='material from PNNL material compendium revion 1. page ....'
my_mat.add_element('W', 1)
my_mat.set_density('g/cm3', 0.003478) |
|
@paulromano I can imagine use cases in model generation where somebody may want access to or use the name attribute for materials in model building efforts, but might not want the full reference to be part of the name. Also, if the reference has a url or doi number in it that would make the name attribute pretty cumbersome to use. |
|
Why not just add a comment with references to the python code? I don't see any scenario where this needs to be propagated into the XML. |
|
@gridley If you were to submit a neutronics calculation for QA/QC during design you would likely hand over all the inputs necessary to run openmc, but not necessarily the python file that generated them. The xml files are ultimately the important files to quality check. I think this is a pretty low overhead addition that would actually add value to the code in terms of adopting it in startups that often outsource QA stuff to consultants. |
|
Hm, I'm not familiar with the requirements of QA or QC, so have to take your word for that. |
It is not the most fun topic out there, but I think every bit of traceability helps. On the BEIS Quality Assurance (QA) modelling: guidance for models document I think OpenMC falls in to the "non Excel" category 😂 |
|
We're going through this process right now with the SPARC design and MCNP. This small addition is essentially born out of expediting that process in the hopes that OpenMC can supplant MCNP as the tool of choice in fusion design (sooner rather than later). |
|
I see, well, my prior comment was simply due to my lack of knowledge on the tasks you all are facing! As long as we're not adding more fields to the C++ data structures that can impact performance, I see no reason against this aside from the possibility that you two will be the only ones using this feature. If that became the case, this could be considered software bloat. So, if doing this for materials, why not do similar for geometry or the like? It seems like any object in the python API should have a field for additional documentation, then. |
|
Also, skimming that QA document, I still don't see why you wouldn't just do QA on the python script that creates the input. If the goal of QA is for someone else to catch errors and have everything be thoroughly documented, the python side is the way to go. |
|
There are just too many processes that happen between reading/writing the text in a python script (which may not contain material definitions that are likely loaded from an external library of previously QA'ed materials either stored as a python file or more likely an xml file) and the materials that show up in the simulation itself. QA is a giant pain and you have to check every "pipe" between your starting point and the end point. If you can cut out some of that process that speeds things up. Plus, when things in the pipes change from time to time they need to be re-qualified. The xml file format for materials seems pretty stable and robust and I don't really see any need for that to change in the C++ layer which makes it a good target for QA'ing once and being done with it more or less. As for everything else in the Python API, I don't think this needs to be done for anything like geometry/settings/tallies etc. Those things would get QA'ed in a separate fashion to the material compositions. Ultimately, I think it would be nice to ship a material database along with OpenMC out of the box so to speak since more or less the first thing I did when starting with OpenMC was create a list of materials and store it for future use. Most of these materials would have standard references so I think it would be important to include them. I'm also against software bloat (I'm not sure I know anyone who's not), but I think you'd see more people than just Jon and I relying on this feature, especially if we add a common materials library. |
|
Thanks for explaining that Ethan. I see where you're coming from now. On the topic of materials, we definitely need to get something similar to Serpent's OK, just looked it up. PNNL 15870. https://www.pnnl.gov/main/publications/external/technical_reports/PNNL-15870Rev1.pdf |
|
Right. That PNNL document was what I was thinking would make up the initial cut at a material library. I've never used Serpent, so I'm not familiar with the |
|
I love this PR and can echo the benefits of making it easier to QA models. @gridley hits on a good question that many have struggled with in the past (revised slightly to make the point): "should we be QA'ing the ASCII input files or the program that made those inputs?" Generally folks prefer to QA the ASCII as part of the engineering QA process, as if you QA the program that made them, then you are in the Software QA space which can be way more involved and time consuming. I'd support making it easier for QA however possible, though I do have 2 reservations (which arent necessarily reasons not to do):
In addition to the above, may I make a proposal though? What if the |
|
I like all the points you're making @gridley and @nelsonag. I'd echo the sentiment that QA'ing the ASCII inputs is the preferred method in the engineering QA process (not that I agree with this necessarily, but it seems to be the way things are right now). Whether or not that is a good enough reason to keep XML around - I don't know, but likely not. As for generalizing the |
|
@nelsonag until you can 100% guarantee that the python script you run, runs the same on my machine as it does yours including your own funky dependencies and environment settings, then I think the xml based input is here to stay. I can be very confident that if I get your nuclear data, your xml file, and the git sha of the version of openmc you used, I can be pretty confident we'll have identical results. That can't be said of the python 'input' which may be sensitive to python versions, dependencies, environment variables etc etc. That being said, the more we can encourage people along the software qa route the better. Further, my experience currently with using OpenMC in a multiphysics environment, an extra bit of python would be really unwelcome and unweildy as part of the running stack. |
|
@eepeterson, @makeclean, I agree with you 100% that the XML is generally preferable for production-grade computations and I am not going arguing that we should be getting rid of it. There is not an existing discussion/issue/etc that I am aware of; I am simply remembering our decision to emphasize the API in the docs over the XML (years ago). That is it and no more, it was likely very over-zealous of myself to even include that based on that! ;-) |
|
A (probably poor) thought on the implementation: this would be more flexible if it were extended to a material.metadata like PyNE ? Some kind of iterable to allow the user to attach whatever and however much information as they see fit. |
|
Also, shall we move discussions (that dont relate to this PR) on things we can do to help strengthen the ability ro do software QA of OpenMC and it's models, and to improve engineering QA of models to a thread on slack/discourse? If we want a dedicated thread, I'd be 100% interested in joining in
…________________________________
From: Andrew Davis ***@***.***>
Sent: Wednesday, April 14, 2021 5:08:08 PM
To: openmc-dev/openmc ***@***.***>
Cc: Nelson, Adam Gregory ***@***.***>; Mention ***@***.***>
Subject: Re: [openmc-dev/openmc] added reference to material class and xml output (#1817)
@nelsonag<https://github.com/nelsonag> until you can 100% guarantee that the python script you run, runs the same on my machine as it does yours including your own funky dependencies and environment settings, then I think the xml based input is here to stay. I can be very confident that if I get your nuclear data, your xml file, and the git sha of the version of openmc you used, I can be pretty confident we'll have identical results. That can't be said of the python 'input' which may be sensitive to python versions, dependencies, environment variables etc etc. That being said, the more we can encourage people along the software qa route the better. Further, my experience currently with using OpenMC in a multiphysics environment, an extra bit of python would be really unwelcome and unweildy as part of the running stack.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1817 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAH5GMZY4L3CWMHHXN2SETDTIYG4RANCNFSM4255IICA>.
|
|
@nelsonag I second the slack/discourse thread. @cjwyett thanks for chiming in! After looking over the link you provided it looks like some of the examples of metadata like units shouldn't really be metadata and are in fact already covered by the existing attributes and methods of the |
|
Good discussion here! A lot of excellent points have been raised, many of which resonate with me. To chime in with thoughts about the future of XML/Python, it's never been a goal of mine to completely eliminate XML and go 100% Python. I personally appreciate (and commonly exercise) the ability to have a set of XML files and a plain Not sure how this fits in to the discussion, but there are already things in the code (namely, depletion) that are effectively Python-only and can't be done with XML alone. With respect to the proposed change here, let me throw out some alternatives and see how quickly they get shot down 😄 One option would be to embed both name/reference information in the m = openmc.Material("awesome metal | This material is from PNNL compendium, page 19728127")As long as model scripts adhere to the convention, it's easy to pull out information on the reference. Another alternative -- have the model script itself write a separate reference information file: references = {}
m1 = openmc.Material(name="petersonium")
references[m1] = "from PNNL compendium, page 02139"
m2 = openmc.Material(name="shimwellium")
references[m2] = "from PNNL compendium, page OX14 3DB"
...
with open("materials_reference.txt", "w") as fh:
for mat, reference in references.items():
fh.write("{mat.id}: {reference}\n")Whether you're doing something like this or using the proposed |
|
Thanks for weighing in @paulromano. Of your two suggestions I'd prefer the first over the second since it doesn't involve writing a separate file just for references which could easily get separated from the XML file that ultimately represents the one source of truth as to what materials make it into the simulation. I also don't particularly want to develop a cross code convention with respect to QA for materials that relies on a separate file. I'm probably still partial to a separate |
|
Only problem I can see (perhaps just for OpenMC DAGMC users) with including long references in the .name field is that this field is used to link materials with the DAGMC tags so it is limited to 28 chars when using in DAGMC. |
|
Let's park this for now, it can always be reopened if needed in the future |
7 participants
Hi all
This is a tiny change to the Material.py file that allows one to add a string reference to a material.
Motivation
Traceable materials have to be a good thing, ideally one would know where the material information such as density and isotopes came from. Adding a reference attribute to the Material class and printing in the xml file facilitates traceability.
before
I would save information on the materials with a comment which would be lost in the xml file
After
With this PR one can save information on the materials with a .reference property
and it goes into the xml file
What do people think, is this a worth while idea.