Repository navigation
Adding support for NCrystal materials in OpenMC - #2222
Conversation
|
I can add to what @marquezj wrote, that since last week NCrystal is also available on conda-forge (since @paulromano suggested this during our meeting). |
|
Thanks for the PR @marquezj! Really excited to see this. Just wanted to give you a heads up I probably won't get to reviewing this until early next week. |
|
@paulromano I added comments concerning the two non-obvious fixmes in the PR. Let us know if you have any specific wishes concerning this (or anything else), and we will try to update the PR. |
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
@marquezj @tkittel Thanks once again for this PR. This change really isn't very intrusive at all and I think with a little restructuring, we might be able to make it even cleaner. Here is my first round of comments, mostly just about naming/style conventions. I haven't had a chance yet to actually try this out but plan on doing so next week.
@ameliajo I was wondering if you might be interested in taking a look at this PR too as our resident thermal scattering expert 😄
|
So @paulromano, unless I overlooked something I believe @marquezj took care of all the changes requested so far (including applying clang-format). |
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
Thanks @marquezj and @tkittel for the updates on this PR. In addition to the comments below, the other thing this PR needs in order to be considered for merging is tests/documentation. For testing, take a look at existing examples in the tests/regression_tests folder. At a minimum, we should have a test that utilizes this new capability and ensures that we get the expected answer. Along with that, we'll need some updates to the CI environment so that we can build against NCrystal -- you can look at tools/ci/gha-install-njoy.sh for an example of how we install NJOY. Adding something similar for NCrystal should be straightforward, and (correct me if I'm wrong) the build time for NCrystal shouldn't be prohibitive to add to CI.
I was going to ask for some example inputs using this so I can test it out, but I think the addition of a regression test will serve that purpose.
For documentation, the only thing that's absolutely required for now is to update the section of our user's guide mentioning optional dependencies.
| //============================================================================== | ||
|
|
||
| class NCrystalRNGWrapper : public NCrystal::RNGStream { | ||
| uint64_t* openmc_seed_; |
There was a problem hiding this comment.
This should probably be in the private: section?
There was a problem hiding this comment.
Well, it technically is since the default access level of classes is private :-)
But we can certainly make an explicit private: section if you prefer.
There was a problem hiding this comment.
Thanks! Yes, we generally follow the C++ core guidelines when possible:
NL.16: Use a conventional class member declaration order
|
FYI we haven't forgotten this, but some other work came in the way. We hope to spend some time on this on Friday. |
|
I just pushed a few updates, mainly to use a new py API method I have just added in NCrystal 3.4.1, which helps getting the basic material composition in Python even for complicated (multiphase/enriched/...) materials. In the rare and somewhat academic case of materials where the same element now appears both as a natural element and as a explicit isotope, we are now using the openmc natural abundancies to break apart the natural element. This is like what we have been doing all along in Geant4 (with the C++ API), where we instead of course were asking Geant4 to provide the natural abundancies. I also added |
|
Outstanding issues are tests, documentation and CI as requested. We will have a look at those tomorrow. |
This was left from a previous implementation. Now it is not needed because NCrystal just takes over all neutron scattering events below a given energy threshold.
Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
…e complicated materials
78324fd to
aca4fcb
Compare
|
@paulromano I added a Update: I guess there will be a few more files to update, like perhaps gha-script.sh, ci.yml, ...). Do you prefer to take care of that, or should we try? |
Included references to the main papers and documentation
|
@paulromano : Is it normal that some of the checks do not start? All the started ones have now passed. Cheers, |
|
@tkittel No, you don't need to worry about that. GitHub is expecting a set of test configurations that does not include the new ncrystal option, so that's why those extra ones are shown. Once this is merged, we'll just change the list of expected test configurations to include the ones with ncrystal included. |
Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
|
Thanks @paulromano , all changes committed as requested. Anything else? Let me know if you want me to do a rebase of the branch on main now, or do any squashing or whatever :-) |
|
Hi @paulromano Despite the status saying "1 change requested", I think we did everything. Let us know if you think more is needed 😄 Cheers, |
|
@tkittel Apologies this fell off my radar -- will take another look soon! |
Merge develop branch to resolve conflicts
Refactor interface between NCrystal and OpenMC
|
@tkittel One final question -- should |
|
I would postpone that for another time, since there is a common issue here with e.g. the Geant4 hooks. Once we eventually look into it, it might anyway be that we go for another solution altogether for this threshold (e.g. a per-material threshold rather than a global one). |
|
Ok, in that case let's change it to be a global constant as opposed to a global variable in the |
Change ncrystal_max_energy to be a global constant
|
Very exciting to us as well, thanks a lot @paulromano for the support and shepherding! 😄 |
Adding support for NCrystal materials in OpenMC
3 participants
After some discussion with @paulromano we cleaned up this version of OpenMC with support for NCrystal. NCrystal is a software library that provides low energy neutron physics to Monte Carlo codes. It was initially designed with Geant4 in mind, but it currently has interfaces with many other codes, and it can be called from Python, C and C++. NCrystal is developed mainly at the European Spallation Source. You can read more about NCrystal in these papers:
X.-X. Cai and T. Kittelmann, NCrystal: A library for thermal neutron
transport, Computer Physics Communications 246 (2020) 106851,
https://doi.org/10.1016/j.cpc.2019.07.015
T. Kittelmann and X.-X. Cai, Elastic neutron scattering models
for NCrystal, Computer Physics Communications 267 (2021) 108082,
https://doi.org/10.1016/j.cpc.2021.108082
X.-X. Cai, T. Kittelmann, et. al., "Rejection-based sampling of inelastic
neutron scattering", Journal of Computational Physics 380 (2019) 400-407,
https://doi.org/10.1016/j.jcp.2018.11.043
or watch the presentation from @tkittel, at ND2022:
https://indico.frib.msu.edu/event/52/contributions/954/
We have used NCrystal together with NJOY to generate a large library of new materials in ACE format (the NJOY-NCrystal library), but calling NCrystal directly allows to introduce in OpenMC additional physics that is not supported by the ACE format.
In particular, NCrystal allows to generate and sample on the fly scattering kernels for solid polycrystalline materials at any temperature. This is done at initialization, based on a human readable description stored in text form in the NCMAT format. For existing ENDF-6 scattering kernels, it allows the direct sampling of S(alpha, beta), which allows for continuous angular distributions (which are still not supported by the ACE format). NCrystal also support oriented materials (single crystals, highly-oriented pyrolytic graphite) and has plug-in support for other physics, currently being used to include small angle neutron scattering and inelastic magnetic scattering. All this would make OpenMC more useful to the neutron scattering community, while adding advanced nuclear data capabilities for the reactor community.
This pull request is planned as an optional feature that could be activated with cmake, using the OPENMC_USE_NCRYSTAL flag after installing and initializing NCrystal. A new method is added to the Material class in Python, to create materials from NCrystal, which takes the composition and density, creates the OpenMC material and associates it with NCrystal.
After reading the xml files, an NCrystal object is created in memory, which provides thermal scattering cross sections and scattering events (for the calculation of the scattering events, the direction of the neutron is a parameter to be able to consider oriented materials). When the thermal scattering cross section is computed, the nuclear elastic scattering component is removed, similarly to what is done in the standard S(alpha, beta) treatment.
There a couple of open topics that we could not decide, and are marked with "fixme" in the code. For instance, NCrystal uses a cache that could be implemented in OpenMC to reduce the computation time. Also, new tags for NCrystal events could be added for tallies. But, overall, we think it is ready for merging upstream.