-
Notifications
You must be signed in to change notification settings - Fork 503
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
Fixes in MicroXS.from_multigroup_flux #3192
Fixes in MicroXS.from_multigroup_flux #3192
Conversation
@@ -327,7 +329,7 @@ def from_multigroup_flux( | |||
xs = lib_nuc.collapse_rate( | |||
mt, temperature, energies, multigroup_flux | |||
) | |||
microxs_arr[nuc_index, mt_index] = xs | |||
microxs_arr[nuc_index, mt_index, 0] = xs | |||
|
|||
return cls(microxs_arr, nuclides, reactions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changes above look good, but would this also do the trick?
return cls(microxs_arr, nuclides, reactions) | |
# ensure microxs_arr has the correct number of dimensions for a one-group cross section | |
microxs_arr = np.expand_dims(microxs_arr, -1) | |
return cls(microxs_arr, nuclides, reactions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work if we keep microxs_arr
as 2D, but it is a lot less obvious to someone reading the code in my opinion, so I'd prefer the current version (explicit is better than implicit).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @paulromano!
Description
This PR fixes two issues in
MicroXS.from_multigroup_flux
. First, as described in #3121, when aMicroXS
object is created using this classmethod, it cannot be written to csv, This was found to be due to the fact that it was creating a 2D array with shape (nuclides, reactions) instead of a 3D array with shape (nuclides, reactions, group) as it should have been. The other issue I observed is that this method modifies the value ofmultigroup_flux
that is passed in, which can be undesirable for the user.One other small fix in here is for the
IndependentOperator
class, which now accepts a normal list of materials (before you had to pass an instance ofopenmc.Materials
).Fixes #3121
Checklist