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

Adds a tolerance for temperatures slightly out of bounds when interpolating #2265

Merged
merged 9 commits into from
Nov 15, 2022

Conversation

joshmay1
Copy link
Contributor

This change is targeted at addressing provided temperatures slightly outside of existing cross section temperatures when in interpolation mode. The allowable distance outside of the existing bounds is specified with the temperature_tolerance attribute of the settings.

For example, if cross sections are available at 300K and 600K and a temperature of 299K is specified, the current code would not allow this as it is unbounded on the lower end. With this change, as long as the difference from the bound to the temperature is within the temperature_tolerance, the loaded cross sections will snap to the bound instead of interpolating. The default value of the temperature_tolerance is still 10K as this parameter is also used in the nearest method for temperatures.

This should close #729.

src/cell.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@lewisgross1296 lewisgross1296 left a comment

Choose a reason for hiding this comment

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

I added some feedback for the changes in response to my earlier comment. Besides that and the documentation changes, which look good, I'm not sure I'm familiar enough with the other source files to give more thorough review. I also do not have write access, so my review sadly will not unblock the PR. Maybe you could request review from someone who has write access?

src/cell.cpp Show resolved Hide resolved
Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement @joshmay1! Sorry it took me a while to review this in full. I have a few small comments but overall it looks like it's in really good shape.

src/nuclide.cpp Outdated Show resolved Hide resolved
src/nuclide.cpp Outdated Show resolved Hide resolved
src/thermal.cpp Outdated Show resolved Hide resolved
src/thermal.cpp Outdated Show resolved Hide resolved
tests/unit_tests/test_temp_interp.py Outdated Show resolved Hide resolved
Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Everything looks good now -- thanks for the updates @joshmay1! I'll wait for your response about updating the check when pushing back temperatures for thermal scattering before I merge this.

@joshmay1
Copy link
Contributor Author

I think it's good to go now @paulromano. It was indeed reading in libraries multiple times for duplicated values, but only when executing the interpolation tolerance I had just added.

@paulromano paulromano merged commit df3334f into openmc-dev:develop Nov 15, 2022
@paulromano
Copy link
Contributor

Thanks @joshmay1!

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.

Handling temperatures just outside the library
3 participants