Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscellaneous updates and improvements from NEA course #2466

Merged
merged 12 commits into from
Apr 24, 2023

Conversation

paulromano
Copy link
Contributor

Last week we held our first in-person OpenMC course in conjunction with OECD/NEA. As usual, when we run these workshops/courses, there is always a long list of little improvements that are identified. I've gone through our list and made a bunch of small fixes/improvements that are contained in this PR:

  • Several Python functions from openmc.lib are missing from our documentation. These have now been added.
  • The DataLibrary class essentially holds a list of libraries, but before required that you access the list through a special .libraries attribute. I've updated that class so that it now acts like a list directly. Along with that, I've added a remove_by_material method that makes it easier to manipulate an existing library (e.g., replacing data for a specific nuclide).
  • The openmc.deplete.Results class requires that you pass the name of the depletion results file. Since this is normally "depletion_results.h5" that is now the default.
  • The openmc.mgxs.EnergyGroups object now accepts a string representing the name of an energy groups structure, e.g. openmc.mgxs.EnergyGroups('CCFE-709'). I also added a __repr__ method for convenience
  • Someone pointed out that openmc.model.pack_spheres always returns the same values because the seed is fixed, which is different than most other functions in our API that sample things randomly. This function has been updated to not use the same seed for every single invocation.
  • Added a check in Plane.from_points for three points that lie along a line.
  • Improved a confusing warning message about a particle filter missing.
  • Small update in CMakeLists.txt to check whether Catch2 is missing as suggested by @yurivict in configure finds Catch2 and then still complains about the missing catch submodule; tests are broken #2449.

@shimwell
Copy link
Member

shimwell commented Apr 13, 2023

Great to see this user driven feedback into the code. I noticed the tests were failing and I was keen to see why.

I think this is due to the file depletion_results.h5 not being found.

This file is now being looked for because the deplete.Results class has a filename that is no longer equal to None and this check means the code tries to open the h5 file.

https://github.com/paulromano/openmc/blob/f23620896d737d74030281d6a09efd3715cfe9fc/openmc/deplete/results.py#L63

One option to get the tests running again would be to replace

results = openmc.deplete.Results()

with

results = openmc.deplete.Results(filename=None)

in a few of the deplete tests, but I'm not sure this is the nicest solution.

@paulromano do you think it is worth removing that if filename is not None part of the deplete.Results.__init__ ?

@paulromano
Copy link
Contributor Author

Thanks for pointing out the failures @shimwell. After looking at the Results class, I think the simplest thing really is just to pass filename=None for the sake of that one test. I've just updated the branch with that fix as well as one other.

Copy link
Member

@shimwell shimwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, will merge tomorrow if there are no objections

@shimwell shimwell added the Merging Soon PR will be merged in < 24 hrs if no further comments are made. label Apr 21, 2023
openmc/data/library.py Outdated Show resolved Hide resolved
Co-authored-by: Gavin Ridley <[email protected]>
@shimwell shimwell merged commit 895d969 into openmc-dev:develop Apr 24, 2023
@paulromano paulromano deleted the nea-workshop-updates branch April 24, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merging Soon PR will be merged in < 24 hrs if no further comments are made.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants