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

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

Open
shikhar413 opened this issue Jul 5, 2024 · 7 comments

Comments

@shikhar413
Copy link
Contributor

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 neams-th-coe/cardinal#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
Copy link
Contributor Author

Tagging @paulromano and @ebknudsen for their opinions on this

@shikhar413
Copy link
Contributor Author

Also tagging @aprilnovak so she is aware of this issue

@ebknudsen
Copy link
Contributor

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
Copy link
Contributor Author

shikhar413 commented Jul 10, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor Author

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?

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

No branches or pull requests

3 participants