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

Gaussian Energy Broadening #2918

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

HunterBelanger
Copy link
Contributor

Description

This PR adds the ability to perform Gaussian Energy Broadening (GEB) on tallies while performing a simulation, instead of needing to do it as a post-processing step. To make this work, I added a new PRN stream for tallies, and this stream is used to sample a new effective energy that is only used for tally scoring purposes.

Due to how the filter system works, I could not find a clean way to add this functionality in the form of a new filter (because the original energy needs to be saved and reassigned after scoring in all bins). Instead, GEB is applied directly to a Tally instance, and all scores associated with a tally will have GEB with the same a, b, and c parameters. Therefore, if you want to simulate multiple detectors, each with different broadening parameters, you will need a different tally instance for each one.

I tested this out on the same problem found in the pulse height tally example notebook, and the results obtained with in-simulation GEB are in great agreement with the post-processing results.

GEB

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 followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@HunterBelanger
Copy link
Contributor Author

So it looks like this is giving slightly different results with event based transport. So far, I am very perplexed by this difference, as the new PRNG stream should only be used in sampling the tally energy. The only thing I can think of is tallying is done in a different order for event based transport ? I will try to look into this.

@cfichtlscherer
Copy link
Contributor

@HunterBelanger thanks a lot for your work here! This is a very useful feature. I know I also had some fights with the event based transport. Let me know if you get stucked.

I was wondering if there are other cases where the tally results need a little reprocessing (that could be incoperated directly in OpenMC) before using them. We could expand your logic here. What do you think @paulromano ?

@HunterBelanger
Copy link
Contributor Author

A little more information on the event-based bug. It appears that in the history-based simulation, there is a particle that is coming into pulse height scoring without any filter matches, while in the event-based simulation the same particle (with exact same initial energy and GEB sampled energy) is coming in with filter matches. Here are the two filter_matches from GDB:

History-Based:

 p p.filter_matches_
{{bins_ = std::vector of length 0, capacity 0, weights_ = std::vector of length 0, capacity 0, i_bin_ = 0, bins_present_ = false},
 {bins_ = std::vector of length 0, capacity 0, weights_ = std::vector of length 0, capacity 0, i_bin_ = 0, bins_present_ = false}}

Even-Based:

p p.filter_matches_
{{bins_ = std::vector of length 1, capacity 1 = {0}, weights_ = std::vector of length 1, capacity 1 = {1}, i_bin_ = 0, bins_present_ = true},
 {bins_ = std::vector of length 0, capacity 1, weights_ = std::vector of length 0, capacity 1, i_bin_ = 0, bins_present_ = true}}

This ends up leading to different FilterBinIter instances constructed in lines 2379 and 2380 in tally_scoring.cpp. From this, it seems that the problem with the event-based test is from a difference in how the filter_matches are stored/cleared in history-based and event-based transport, though I have not found where yet.

@cfichtlscherer , Paul had a similar idea, that there might be other types of tallies which could benefit from a similar logic (which is why I ended up making a specific tallying stream for the PRNG). However, I am personally unaware of any other type of tally that would require the sampling of a random quantity for scoring purposes.

@HunterBelanger
Copy link
Contributor Author

@cfichtlscherer , my last commit removed 2 lines from your original pulse height tally scoring function. On my system, this gets the tests to pass in history based and event based transport. Looking at the code, it seems like having the continue there was a bug ? Because if we continue there, we never get to the end of the loop to clear the filter matches on the particle. I just want to make sure that this indeed shouldn't be there.

@cfichtlscherer
Copy link
Contributor

Dear @HunterBelanger I am sorry for my late reply. Nice that you managed to find out what was going wrong there. But I am not sure if we can simply remove these two lines. This logic is part of basically every tally_scoring function. When I wrote this piece of code I actually "copied" the style from the existing tallies. I'm a little worried that I don't have enough of an overview of the situation here and would therefore ask @paulromano to check it again.

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