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

Use temperature bounds in multigroup mode temperature #3059

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

GiudGiud
Copy link
Contributor

@GiudGiud GiudGiud requested a review from aprilnovak June 24, 2024 21:34
@GiudGiud GiudGiud self-assigned this Jun 24, 2024
@GiudGiud GiudGiud requested a review from nelsonag as a code owner June 24, 2024 21:34
@aprilnovak
Copy link
Contributor

@meltawila can you check your Cardinal simulation against this newer OpenMC branch? You'll want to do

export OPENMC_DIR=/path/to/guillaumes/openmc/checkout
cd cardinal
rm -rf build install
make -j4

and then try running your models to see if this picks up the temperature range correctly.

@@ -95,6 +95,32 @@ void Mgxs::metadata_from_hdf5(hid_t xs_id, const vector<double>& temperature,
settings::temperature_method = TemperatureMethod::NEAREST;
}

// Determine actual temperatures to read -- start by checking whether a
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a place in OpenMC to place this which can allow us to consolidate this code and the identical lines from nuclide.cpp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like a util header? I'm not sure which one, maybe if there is one for std::vector

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could put a utility function in settings.h that sets temps_to_read?

Copy link
Contributor Author

@GiudGiud GiudGiud Jun 24, 2024

Choose a reason for hiding this comment

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

with any luck we might be able to include from line 98 to 178 in this utility. @meltawila do you want to give it a try?

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