Resolve conflict with weight windows and global russian roulette

[GuySten]

Description

Currently the global neutron russian roulette (which is used when survival biasing is turned on) conflicts with the application of weight windows.
As suggested in #2773 (comment), this PR makes global neutron russian roulette apply only when neutrons are outside of the weight windows mesh boundaries.

Fixes #2773

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)

[pshriwise]

Thanks @GuySten! You are a machine. Some thoughts to consider here in the comments.

[GuySten]

I've restructured the code to be more backwards compatible.

[GuySten]

I've further improved the code structure according to your suggestion.
I will have to dig into why the tests fail.

[GuySten]

This PR is now ready for another review iteration.

[pshriwise]

I think we're getting closer here. I made a suggestion for simplified logic in the collision functions.

There are also some checks on particle state inside get_mesh_bin that feel a little out of place. Not incorrect really, but the don't seem to fit into the purpose of that function. There might be some further refactoring we can to to make that a little cleaner.

[GuySten]

@pshriwise, I've incorporated your suggestions and this PR is ready to review.

[pshriwise]

After some testing, there's more work to be done here to clarify the difference between a weight window that is not found and the case where a weight window is found but unset. I'll add a post here with a solution later today.

[pshriwise]

I tried applying this to a similar problem to the one presented in #2773. It's a block of Tungsten with an incident planar source of neutrons. All boundaries are reflective except the positive X plane, which is a vacuum boundary condition. These are short runs with 10k particles, 10 batches.

When I tried it as of 57b5d27, the survival biasing was still limiting the effectiveness of the weight windows because unset weight windows (different from the search for a weight window failing) were also resulting in WeightWindow::is_valid returning false and pushing us into the all to apply_russian_roulette. I'm glad I was using the MAGIC method for weight window generation in this case or I likely wouldn't have noticed this!

At any rate, what we need to separate here is the cases where a weight window is unset in the WeightWindows object vs. not being able to find one due to energy limits, a particle being outside the mesh, etc. The latest commit I pushed here takes care of that with an extra flag emitted from search_weight_window and get_weight_window. Happy to discuss the design of that further, but from the images below we can see we're now getting the effect we desire.

Weight windows, no survival biasing (same in 57b5d27 and a53c066)

Weight windows w/ survival biasing on 57b5d27

Weight windows w/ survival biasing on, a53c066

[GuySten]

@pshriwise, can you add the last test (weight windows w/ survival biasing) as a regression test?

[pshriwise]

@pshriwise, can you add the last test (weight windows w/ survival biasing) as a regression test?

Yeah that's good call. I'm hesitant to make it a regression test as it's going to be pretty sensitive to the prn stream. I'm thinking at least a coarse check that turning on survival biasing at least doesn't reduce the number of mesh bins with tallied results. I can't think of a reason that it would.

I'll also look into the now-failing regression test. Something must be different with the control flow without survival biasing on now.

[pshriwise]

Okay I finally had some time to dig into the failing test and I found, well, a number of things.

First, I noted that if I omitted the photon weight windows from the regression test that it would pass on both develop and this branch. Then I realized that there are actually two sets of neutron weight windows in that test (which I've corrected in #3798) and this was partially the cause of the problem. In the previous apply_weight_window function we weren't differentiating between a weight window not being found and one being unset, so in the loop searching for a weight window

openmc/src/weight_windows.cpp

Lines 69 to 76 in 96383fc

	bool in_domain = false;
	// TODO: this is a linear search - should do something more clever
	WeightWindow weight_window;
	for (const auto& ww : variance_reduction::weight_windows) {
	weight_window = ww->get_weight_window(p);
	if (weight_window.is_valid())
	break;
	}

it would note the weight window isn't valid (now what we consider to be unset) and continue to search in the second set of weight windows, sometimes finding a valid window to use. Now that we accept a weight window if it is unset or not, this changes the behavior of the test case -- but only because there are two overlapping sets of neutron weight windows! So, as I mentioned before I've corrected this test in #3798.

It's not really valid to have overlapping sets of weight windows like that. We may want to enforce that only one set of weight windows is allowed per particle type, but I don't want to rule out any creative use cases of multiple (non-overlapping) weight window meshes either.

tl;dr: once #3798 goes in, we should be able to update this branch and expect tests to pass.

[GuySten]

@pshriwise, now the only thing missing is to turn your tungsten model into a unit test where we check that more bins are scored to with weight windows and survival biasing compared to weight windows alone.

[pshriwise]

Everything looks good after the updates. Placing a hold here for a bit while I look at a couple more test scenarios.