Conversation
|
@munkm @ilhamv should I add support for energy weight windows in this PR as well or do a follow up PR for that? Also, the added documentation to the website in this PR is pretty sad, but I was unsure as to whether I should expand the docs around all of the |
|
I think it's fine to do energy based WWs in a separate PR. I do think you want to add in at least minimal documentation to this PR (I will do my best to take a look to do a review in a few days). If you want to do expanded documentation you could make a second PR that is dependent on this one being merged in (?). |
|
@munkm I will create a separate PR to add the docs to so as to not clutter this one |
|
@ilhamv I am not sure what is causing the regression tests to fail, but I think (?) that since I did not add a regression test the |
|
This looks great, @nglaser3. It looks like the WW is called not only at every collision, but also at every surface crossing, which is good, I think. |
|
Hi Caleb (@shac170), I invited you to review/take a look at this PR on re-introducing weight windows into the refactored MC/DC. |
|
@ilhamv thank you for the review, I will work on addressing your comments! |
Co-authored-by: Ilham Variansyah <variansi@oregonstate.edu>
|
@ilhamv, I believe I have updated the PR with your comments! Below is a summary of the changes (enumerated from your comment):
|
|
@nglaser3 - I just redesigned the original weight roulette into Minor: I added |
|
@nglaser3 - Is it true that the way it is set up now, only one of the split particles will be rouletted (when applicable)? Should all of the split particles be rouletted instead? |
|
Thanks @ilhamv for fixing up the roulette logic! I think it makes sense to be able to support both weight windows and global roulette as opposed to silently skip over the global roulette, even though it probably doesn't make sense for a user to set both Yes, I did not even realize that if a split occurs, the split particles are not then checked for roulette, only the parent particle. I will change this so that all particles are roulette |
|
@ilhamv, I have updated the weight window procedure to roulette all split particles not just the parent! |
|
For some reason, the regression test answer still needed to be revised... I have a question on the WW procedure. Can we split excessive-weight particles right into the targeted weight? (this would be similar to the split-roulette procedure in the population control) |
|
@ilhamv I totally missed this, sorry! No, I do not think we could since the quotient of the particles current weight and the target weight from the weight window is no guaranteed to be an integer (to determine how many splits should occur). |
|
Here is how we do the splitting-roulette procedure in the population control:
Or, in the actual implementation, after we calculate Note that this procedure works both for === Do you think this would work for WW? |
Summary of changes
This PR adds mesh-based weight windows to MCDC. In the current state, only spatial and energy weight windows are supported. Additionally, the particle "passes through" a weight window on collision or surface crossing, as opposed to whenever a particles crosses into a new mesh bin.
Weight windows are stored as three separate 1D arrays of floats:
lower_weights,target_weights,upper_weights.To test the additions, I added a few unit tests to
unit/transport/technique/weight_windows.py.Types of changes
Developer Checklist
Associated Issues and PRs
Associated Developers