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

Fix bug for distributed temperatures without distributed materials #1178

Merged
merged 1 commit into from
Mar 5, 2019

Conversation

smharper
Copy link
Contributor

When we call get_temperatures to find out what temps are assigned to each nuclide, there's a bug that occurs related to distributed temperatures. We correctly account for the case where the cell has one temperature and many materials, or N temperatures and N materials, but we missed the case where it has many temperatures and only one material.

@smharper smharper added the Bugs label Feb 25, 2019
@paulromano
Copy link
Contributor

@smharper Seems like this has been a recurring problem -- can you add a test for this case?

@smharper
Copy link
Contributor Author

smharper commented Mar 4, 2019

I just looked into adding a test. Unfortunately, I don't think we'll be able to catch this bug with any tests until we start using pointwise cross sections with multiple temperatures for our test suite. For example, the multipole regression test should trigger this condition (distribtemps without distribmats), but it doesn't matter because i_temp can only ever be zero.

} else {
for (double sqrtkT : cell->sqrtkT_)
cell_temps.push_back(sqrtkT*sqrtkT / K_BOLTZMANN);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like determining cell_temps ought to be done as a separate loop above the loop over cell->material_.size() (line 165). Effectively in each case, we're iteratating over cell->sqrtkT_.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So populate the cell_temps array before the cell->material_.size() loop? I think that way we could end up with loading more temperature data than needed. If every cell instance has a different material and different temperature, then each material only needs to load one temperature.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, you're right. I felt like I might be missing something!

@paulromano paulromano merged commit d6ee835 into openmc-dev:develop Mar 5, 2019
@smharper smharper deleted the distribtemp_bug branch March 5, 2019 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants