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 2 commits into
base: develop
Choose a base branch
from

Conversation

GiudGiud
Copy link
Contributor

@GiudGiud GiudGiud commented Jun 24, 2024

refs #2402

@meltawila

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

For the last item, I'm moving code from a file to another so I should be fine, the code will still be covered by existing testing

@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.

src/mgxs.cpp Outdated
@@ -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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gave it a shot

@GiudGiud GiudGiud force-pushed the PR_mg_T branch 3 times, most recently from 3d707b1 to 5c4ce11 Compare August 29, 2024 21:22
@GiudGiud GiudGiud force-pushed the PR_mg_T branch 2 times, most recently from 822c9cc to 8d74059 Compare August 29, 2024 21:33
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.

2 participants