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 bug fixes #1948

Merged
merged 5 commits into from Jan 17, 2022
Merged

Conversation

paulromano
Copy link
Contributor

@paulromano paulromano commented Jan 12, 2022

This PR has a few small bug fixes:

  1. There's a fix for S(a,b) name suggestions instead of applying a guess #1945 so that you can pass table_name when caling ThermalScattering.from_njoy and it won't try to guess a name
  2. A user noticed that when they tried to run a depletion calculation and specify cross sections using Materials.cross_sections (but not setting OPENMC_CROSS_SECTIONS) they ran into an exception. It turns out that the Operator._get_nuclides_with_data method always tried to find what nuclides have neutron data by just looking at the environment variable but not Materials.cross_sections. This has been fixed in this PR and I've tried to better encapsulate the logic for finding cross sections / depletion chains into single places (the _find_cross_sections and _find_chain_file functions).
  3. There's a fix for model.run() can fail to return the statefile #1939

@pshriwise
Copy link
Contributor

As I understand it, this doesn't quite fix the problem described in #1945.

The problem in the issue is that the user could not successfully reference the new thermal scattering data in material.add_s_alpha_beta because the name was not present in openmc.data.thermal._THERMAL_NAMES but the new name ("c_Graphite_20p") was close enough to an existing data name in _THERMAL_NAMES that the openmc.data.thermal.get_thermal_name function was returning a valid, but unintended, dataset by providing a close match. I've updated that issue with a little more info and proposed fix that we could include in a separate PR.

So, a matter of using already generated data rather than creating a dataset with custom names. Though the user did indicate in the forum thread that they also had to adjust some things script to ensure the right name was being applied to their data, so probably added some fixes similar to the ones you've applied here too.

@paulromano
Copy link
Contributor Author

@pshriwise definitely did not realize the issue was with Material.add_s_alpha_beta. Now that I see that, I agree it doesn't make sense for us to second guess the user's choice here. I've added a commit that takes away the guessing in the add_s_alpha_beta method altogether.

Copy link
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

This looks good to me @paulromano. Thanks!

@pshriwise pshriwise merged commit 0a82da0 into openmc-dev:develop Jan 17, 2022
@paulromano paulromano deleted the more-bugfixes branch January 17, 2022 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants