openmc-dev / openmc

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

A proposal for consistent behavior between `Results.export_to_material` and `StepResult.get_material` #3126

Open pshriwise opened 2 months ago

pshriwise commented 2 months ago

Description

There are currently two ways to construct Material Python objects from depletion results:

I understand that they serve different purposes (one resulting material definition is intended for neutron transport and the other is a description of the depleted material composition from the depletion results). The outcome is that we have two different ways of obtaining Material objects during depletion post-processing.

As StepResult.get_material is used in many other places within Results, I propose that we change Results.export_to_materials to use get_material under the hood. To maintain the current behavior, I'll propose a method be added to Material that removes nuclides without transport cross-sections based on the current configuration. (I think the nuclide removal method is best placed there, but others may have more thoughts on that). This method can be called on each resulting material object and should result in the same material compositions returned by the method now.

I think this flag could replace the nuc_with_data capability in most cases, simplifying this method. It also places logic for material construction in a single place, which is generally preferrable for codebase maintenance.

New proposed signature:

    def export_to_materials(self, burnup_index: int, transport_only: bool = True, path: PathLike = 'materials.xml'):
        materials = openmc.Materials(path)
        step_result = self[i]
        materials_out = openmc.Materials([step_result.get_material(str(m.id)) for m in materials])

        if transport_only:
            for m in materials_out:
                m.remove_non_transport_nuclides() # what a horrible method name I've chosen

        return materials_out

Alternatives

Perhaps changing the name of the export_to_materials to something like export_to_transport_materials would perhaps clarify the purpose of this method, but it would not solve the underlying inconsistency in material generation from depletion result files.

Compatibility

If the nuc_with_data option is removed, this would be a breaking change. This one is optional I'd say though. The method can still be refactored to use get_material without any change to this method signature in theory. I simply prefer the option of a flag I've suggested above. So, TBD.

yardasol commented 1 month ago

Patrick, I think moving the functionality in the depletion module to remove nuclides with no cross section data to the Material class is a great idea.