Statistical weights in IndependentSource

[zoeprieto]

Description

This is a solution to issue #3194. It adds the capability to set the value of the statistical weights for an IndependentSource. Especially useful when you have multiple sources with differences in several orders of magnitude of intensities.

Fixes #3194

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)

[gonuke]

Thanks for this contribution @zoeprieto. I'm not sure why weight is a distribution to be sampled and not just a constant like strength. Your reasoning for this change as described in #3194 seems to suggest that it should be a constant for each IndependentSource

[zoeprieto]

Thank you for your response @gonuke! I had thought that for problems where you want to model a single source, it might be useful to make a weight distribution. But on second thought, that would be useful if I could simulate it coupled to another variable, such as energy. But as it is not implemented, the option should be only 1 value. I will work on that :)
Thanks again.

[gonuke]

This looks good to me.

I wonder about documenting the relationship between strength and weight. The most logical use would be for one of them to always be 1. That is, if you have multiple sources, the easiest to understand is all the sources have weight=1 and different strengths, or all the sources have strength=1 and different weights.

If they had different strengths as well as different weights, it will be a little tricky to interpret.

[zoeprieto]

Yes, I absolutely agree, they should not be used simultaneously.
I updated the documentation. Let me know what you think.

[gonuke]

This discussion of documentation made me think of a different approach to this (sorry) - what if there was an option in the settings to just treat the strengths as weights. Then a user could define the relative strengths just as they do now, but the sampling between the distributions would be handled differently just by changing that option.

(Note, I'm particularly interested in this topic right now because I have a student working on #3104 )

[zoeprieto]

I think what you are proposing is a better solution. Especially because there is no option to make a mistake by setting the strength variable simultaneously with weight. I have already implemented this as a first approximation. Let me know what you think.

[gonuke]

This looks a lot simpler. Interested in various opinions on the best name for this. I think strengths_as_weights makes a little more sense, but there may be a lot of different thoughts?

[zoeprieto]

Thanks for the comments, I have renamed the variable to strengths_as_weights and fixed the changes you pointed out.

[gonuke]

This looks good to me - it will be up to @pshriwise or @paulromano to give it a final review/approval.

[paulromano]

Thanks @gonuke. I'll put this on my list and should be able to get to it this week.

[paulromano]

Thanks for this PR @zoeprieto! I made a few changes/fixes/improvements here as follows:

• Rather than this being something specific to IndependentSource, I've raised the logic for applying the source strength into the sample_external_source function so that it works with any source class
• There is a piece of code in tally.cpp where we apply the source strength to tallies as a normalization after the run is done. Since the strength is applied during transport for this option, I added a conditional to ensure that the strength is not applied twice (which would result in wrong results). I also added a new test to check for this.
• I changed the name to uniform_source_sampling, which I think more clearly expresses to the user what it is about.
• I used your ParticleList class in the tests to simplify them 😄

If you and @gonuke are satisfied with these changes, this should be good to merge.

[zoeprieto]

I think the changes are great @paulromano . Thanks for generalizing it to all type of sources and sorry about the normalization of the tallies, I missed it.

[gonuke]

I'm on the fence about uniform_source_sampling because it doesn't convey anything about bias or weight adjustment. I worry a user may interpret this as uniform strength and not a biased uniform sampling... not sure about a better name, though.

[paulromano]

a user may interpret this as uniform strength and not a biased uniform sampling

I put "uniform" and "sampling" in the name so as to avoid that interpretation. The docstring in Settings is also pretty explicit about what it does, which should also help avoid misinterpretation, although if you think it could be clearer I'm open to suggestions.

[paulromano]

@gonuke Are you OK with merging this as is?

[gonuke]

I'm OK with this as is