openmc-dev / openmc

OpenMC Monte Carlo Code
https://docs.openmc.org
Other
750 stars 483 forks source link

Fission site bounding box reporting when threshold fissionable site rejection sampling error is triggered #3073

Open shikhar413 opened 2 months ago

shikhar413 commented 2 months ago

Description

2916 fixed an error check that was previously not properly implemented in IndependentSource::sample, where the variables n_accept and n_reject are now properly defined as static variables. Previously, only n_reject was defined as static, so the error check for checking the fraction of rejected particles was not being hit even though it should have been. What this means for some legacy models now is that if the independent source distribution bounding box was defined over a large domain of non-fissionable material, now the model needs to be updated to encompass a narrower domain that only spans the fissionable domain. The emergence of this error message is already happening for some Cardinal models through the version update in OpenMC (see https://github.com/neams-th-coe/cardinal/pull/903). This requires inspecting the geometry and trying to determine the minimum bounding box of fission sites, which can be quite tedious to do for more complicated models. Instead, what might be preferred here is if the error message for threshold percentage of rejected particles over fissionable areas is triggered, then OpenMC could also report the minimum bounding box from the fission sites that were samples so far

Alternatives

None

Compatibility

No changes to the core functionality, just more useful error reporting

shikhar413 commented 2 months ago

Tagging @paulromano and @ebknudsen for their opinions on this

shikhar413 commented 2 months ago

Also tagging @aprilnovak so she is aware of this issue

ebknudsen commented 2 months ago

In essence, I suppose this is simple enough to implement. One would simply keep a running log of the most extreme but accepted source site spatial coordinates. On the other hand, such corners would always be slightly inside a "fissionable-enclosing" bounding box, so depending on the use-case manual "fiddling" might be necessary anyway.

shikhar413 commented 2 months ago

Yeah I think the idea here is that even if you have the fissionable enclosing box a bit smaller than the "ideal" size, if you run enough inactive cycles then it shouldn't affect your "converged" fission source distribution for use in the active cycles

shikhar413 commented 2 months ago

The only thing I need to dig into is to figure out whether the sampled sites are defined for each MPI process, and whether some MPI communication is needed to get the bounding box defined over all MPI processes

paulromano commented 2 months ago

@shikhar413 If you running into problems with a model where too many sites are getting rejected, my suggestion is to simply turn off the fissionable constraint on the source since you need to converge the source distribution anyway for a k-eigenvalue calculation. At worst, it means you would need one extra batch to converge your source.

Rather than doing something fancy with finding the bounding box of fissionable materials, I think an easier "fix" if you wanted to still use this functionality when the rejection fraction is low is to have the rejection fraction be user-configurable.

shikhar413 commented 2 months ago

That makes sense, that would be a lot more straightforward of a "fix".

By the way, when trying to run with turning the only fissionable constraints off and running in parallel, I see " WARNING: The shared fission bank is full. Additional fission sites created in this generation will not be banked. Results may be non-deterministic." warning on the first batch. Can this warning be ignored?