Fixed volume allocation in openmc.deplete.Operator._differentiate_burnable_mats

[awgolas]

When the variable diff_burnable_mats=True was specified when calling openmc.deplete.Operator(), the volume of each material instance was specified as the same as the non-differentiated burnable materials, e.g, if the given burnable material had 3 instances:

<material depletable="true" id="10001" name="fuel" volume="435">
...
  </material>

The diff_burnable_mats=True would produce differentiated burnable materials:

<material depletable="true" id="1" name="fuel" volume="435">
...</material>

<material depletable="true" id="2" name="fuel" volume="435">
...</material>

<material depletable="true" id="3" name="fuel" volume="435">
...</material>

This changed the material instance creation such that the sum of the differentiated material volumes is the same as the original undifferentiated material volume.

[liangjg]

@awgolas Yes, originally I thought the users specify the volume as the volume of a single instance (not the total volume) when they are going to differentiate the material. That's why the heavy_metal is accumulated, in L345.

If we want to change this logic -- assume the users are specifying the total volume, I suggest changing the differentiated materials themselves when they are cloned (L312).

[drewejohnson]

Thanks for pointing this out. I understand the confusion here, as the documentation isn't clear how the volumes are treated for the cloned materials. Both approaches I think are equally valid, e.g.

1. User-specified volume is for a single instance. Cloned materials copy the volume as well
2. User-specified volume is the total volume this material occupies. Cloned materials take an equal share of the volume (as you proposed)

I'm not convinced that either is worse, but our documentation should indicate what method we choose.
If you want to proceed with this PR, please make sure the test suite passes. The issue appears to be the materials don't know how many instances when you scale the volumes

[awgolas]

@liangjg
I wasn't sure what volume was supposed to be specified at first. I had originally just used a script which used the stochastic volume calculation results in order to determine the total volume of the fuel in the assembly. I had assumed that diff_burnable_mats also differentiated the volume up as well.

When I've created geometries with fuel cells in which the volume of the fuel cell region is not constant, I've used the same material id in each case. If I were to run a depletion calculation in which the material volume specified is equal to the total volume of the material, does OpenMC perform the nuclide accumulation calculations as if each cell volume is equal to the total volume of the material?

@drewejohnson
Is it best practice to specify different material IDs if the fuel cell volume varies? If so, I think I may have just created a problem which didn't exist. It's also worth noting that mat.volume/mat.num_instances would not solve that problem either. However, I think that this would benefit if there was a check for consistency between the total volume and the differentiated material volume. I can close this and resubmit the problem into the issue section if you think that's appropriate.

[liangjg]

@awgolas I agree it is more self-consistent to assume the input volume is always the total volume, no matter whether the differetiation is on or off.
But I think it is better to make changes in the function _differentiate_burnable_mats(), so that the update of volume is only performed when differetiation is on and geometry.determine_paths() is called (that's why your test fails). Code changes are like the following:

--- a/openmc/deplete/operator.py
+++ b/openmc/deplete/operator.py
@@ -309,7 +309,12 @@ class Operator(TransportOperator):
             # Assign distribmats to cells
             for cell in self.geometry.get_all_material_cells().values():
                 if cell.fill in distribmats and cell.num_instances > 1:
-                    cell.fill = [cell.fill.clone()
+                    mat = cell.fill
+                    if mat.volume is None:
+                        raise RuntimeError("Volume not specified for depletable "
+                                           "material with ID={}.".format(mat.id))
+                    mat.volume /= mat.num_instances
+                    cell.fill = [mat.clone()
                                  for i in range(cell.num_instances)]

[drewejohnson]

@liangjg has made excellent points and I agree with their proposed implementation. If you want to adjust the PR accordingly, please do so. Otherwise we can raise this as a separate issue / PR later

[drewejohnson]

Thank you for this! I will put up a PR shortly with some changes to the documentation 👍