Approximate multigroup velocity #3766
Conversation
GuySten
requested review from
paulromano and
pshriwise
and removed request for
nelsonag and
paulromano
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
@nelsonag would love to get your thoughts on this one!
|
Why not tally a 1/v based on the group midpoint energy (I assume logarithmic midpoint is best) if no specific data is present? This could be done void or not. |
|
Do you mean instead of defaulting to speed 0 when 1/v data is missing using velocity that match the logarithmic energy midpoint of the group? |
|
Yep!
…On Fri, Feb 6, 2026, 3:10 PM GuySten ***@***.***> wrote:
*GuySten* left a comment (openmc-dev/openmc#3766)
<#3766 (comment)>
Do you mean instead of defaulting to speed 0 when 1/v data is missing
using velocity that match the logarithmic energy midpoint of the group?
—
Reply to this email directly, view it on GitHub
<#3766 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH5GM276B2D4K5FJGJCUMT4KT7NLAVCNFSM6AAAAACT4QSIHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNRSGU4DENBQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Removed validation for group edges in the MGXS class.
GuySten
changed the title
|
@nelsonag, can you look at this PR again? |
|
I’m very interested in this to support use cases in Cardinal. Is there any testing/support I can provide to help see this PR merged? |
|
Currently this PR waits for a review of either @nelsonag or @paulromano. |
nelsonag
left a comment
nelsonag
left a comment
There was a problem hiding this comment.
Two highly-related comments: why not generalize for any MGXS without 1/v data present?
include/openmc/mgxs_interface.h
Outdated
| vector<double> energy_bin_avg_; | ||
| vector<double> rev_energy_bins_; | ||
| vector<vector<double>> nuc_temps_; // all available temperatures | ||
| vector<double> void_velocities_; // velocity of particles in void regions |
There was a problem hiding this comment.
Why not expand this capability to support when MGXS are not provided in the library? That is, since inverse-velocity is optional in the mgxs.h5 file there will be non-void materials that also do not have 1/v defined. The same strategy you use here can be used for that case.
src/mgxs_interface.cpp
Outdated
| for (int i = 0; i < energy_bins_.size() - 1; ++i) { | ||
| double e_min = std::max(energy_bins_[i], 1e-5); | ||
| double e_max = energy_bins_[i + 1]; | ||
| double v = C_LIGHT * std::log(e_max / e_min) / |
There was a problem hiding this comment.
Have you considered the pros/cons of computing this at tally time vs pre-computing? This makes more sense when expanding this to support the tally of 1/v for any material without 1/v defined where you could end up storing this on many many mgxs sets, thus using more RAM than makes sense.
|
@nelsonag, I've implemented your suggestions and now the code is alot simpler. Now the approximate inverse velocity data is stored only once. |
This reverts commit fe7e5a3.
4 participants
Description
Currently, void regions in multigroup mode makes openmc crash (#3723).
This PR implements the following changes:
Checklist
I have made corresponding changes to the documentation (if applicable)