Add implicit complement and default material properties #57

ahnaf-tahmid-chowdhury · 2025-11-04T13:46:32Z

This PR introduces two new features to the Model class:

1. Implicit Complement Material

The implicit_complement_material property allows getting and setting the material of the implicit complement volume (the volume with the 'Graveyard' material).

The implicit_complement property has been added to easily access this volume.
The implicit_complement_material property getter finds the material of the implicit complement.
The implicit_complement_material property setter allows changing the material of the implicit complement. A _comp suffix is added to the material group name to indicate that it is the implicit complement material.

2. Default Material

The default_material property allows setting a default material for all volumes that do not have a material assigned.

When a default_material is set, all volumes without a material are assigned the specified material.
A marker group of the form pydagmc:default_material:<material_name> is created to indicate the default material.
When the default_material is unset, the marker group is deleted, but the volumes retain the assigned material.

Tests have been added for both features to ensure they work as expected.

NOTE: Used AI agents for fast coding 🙂

codecov · 2025-11-04T13:48:54Z

Codecov Report

❌ Patch coverage is 96.22642% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.98%. Comparing base (b382289) to head (dd8b1e7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/pydagmc/dagnav.py	96.22%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #57      +/-   ##
==========================================
- Coverage   97.08%   96.98%   -0.11%     
==========================================
  Files           2        2              
  Lines         446      497      +51     
  Branches       48       65      +17     
==========================================
+ Hits          433      482      +49     
- Misses         10       11       +1     
- Partials        3        4       +1

Flag	Coverage Δ
unittests	`96.98% <96.22%> (-0.11%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gonuke

I'm not sure all the assumptions and conventions are robust to the rules for how the implicit complement and its material are used in practice. I wonder if we've every written down the expectations for how this information is used in codes? @pshriwise ?

gonuke · 2025-11-04T17:45:51Z

src/pydagmc/dagnav.py

+        Returns the implicit complement volume.
+
+        The implicit complement volume is identified as the volume
+        that has the material 'Graveyard'.


I don't think it's true that a volume in the mat:Graveyard group is necessarily the implicit complement.

Right. In most/many cases the .h5m doesn't have an implicit complement volume -- it typically gets built at runtime. It wouldn't be difficult to construct one using PyDAGMC, however.

It might be beneficial to do this even? One of the challenges we have with the implicit complement material assignment is that the volume doesn't exist yet when we load the file and parse metadata leading to a sort of chicken and egg problem here. If we create the volume in PyDAGMC then we can assign the material easily enough.

One issue I can see with that is keeping track of which volume is the implicit complement when round-tripping the mesh to and from disk. Is marking the implicit complement part of the file spec?

Im pretty sure it's not part of the file spec because it's intended to be reconstructed as necessary, potentially with every read.

What happens when the build_obbs tool is run on a file? Is the implicit complement omitted and run later?

gonuke · 2025-11-05T01:52:53Z

src/pydagmc/dagnav.py

+        # First, find and unset any existing implicit complement material group
+        for group in self.groups:
+            if group.name and group.name.startswith('mat:') and group.name.endswith('_comp'):
+                group.name = group.name.removesuffix('_comp')
+                break


If this group is empty, it should probably just be deleted

gonuke · 2025-11-05T01:53:49Z

src/pydagmc/dagnav.py

+        if material_name is not None:
+            # Find the group for the new material
+            material_group_name = f"mat:{material_name}"
+            if material_group_name in self.groups_by_name:


If this group exists and is not empty, is it safe to just rename it like this?

gonuke · 2025-11-05T01:57:27Z

src/pydagmc/dagnav.py

+        is_comp = self.material_group and self.material_group.name.endswith('_comp')
        self._set_metadata_group(self._material_prefix, name)
+        if is_comp:
+            self.material_group.name += '_comp'


Doesn't this bypass some of the care you used to set the implicit complement material above?

gonuke · 2025-11-05T02:00:08Z

src/pydagmc/dagnav.py

-        if val.lower() in self.model.group_names:
+        current_name = self.name
+        if val.lower() in self.model.group_names and val.lower() != current_name.lower():


I think it's better to just return early if current_name.lower() == val.lower() and then test for the case that raises an exception

Also we should compare val.lower() to the lowercase version of each group name if that's the precedent we're going with here. Could get some false negatives in the current state.

Different question: do we actually assume that group names are case-insensitive? Or should we NOT use lower() at all?

Good point. In OpenMC, lowercase comparison is how we perform comparisons against materials when names are used for assignment. That doesn't have to be the case for all codes that might use names for assignment (though I'll say I think it's worked pretty well for us so far).

To date I believe OpenMC is the only one that allows assignment by name.

pshriwise

Thanks for submitting this @ahnaf-tahmid-chowdhury. There's some cleanup to do here. This will be nice to have for those interested in this capability I think!

pshriwise · 2025-11-05T05:13:53Z

src/pydagmc/dagnav.py

+    @property
+    def default_material(self) -> Optional[str]:
+        """
+        The default material for volumes without a material.
+        """
+        for group in self.groups:
+            if group.name and group.name.startswith('pydagmc:default_material:'):
+                return group.name.split(':')[-1]
+        return None
+
+    @default_material.setter
+    def default_material(self, material_name: Optional[str]):
+        """
+        Set the default material for volumes without a material.
+        """
+        # First, remove any existing default material marker
+        for group in self.groups:
+            if group.name and group.name.startswith('pydagmc:default_material:'):
+                group.delete()
+                break
+
+        if material_name is not None:
+            # Assign the material to all volumes without one.
+            for volume in self.volumes_without_material:
+                volume.material = material_name
+            # Create a marker group to store the default material name
+            self.create_group(name=f'pydagmc:default_material:{material_name}')
+


Not a bad concept to introduce, but this is out of scope for the PR. Let's keep it to one thing at a time and focus on the implicit complement.

pshriwise · 2025-11-05T05:20:11Z

src/pydagmc/dagnav.py

-        if val.lower() in self.model.group_names:
+        current_name = self.name
+        if val.lower() in self.model.group_names and val.lower() != current_name.lower():


Also we should compare val.lower() to the lowercase version of each group name if that's the precedent we're going with here. Could get some false negatives in the current state.

pshriwise · 2025-11-05T05:20:21Z

src/pydagmc/dagnav.py

+        Returns the implicit complement volume.
+
+        The implicit complement volume is identified as the volume
+        that has the material 'Graveyard'.


It might be beneficial to do this even? One of the challenges we have with the implicit complement material assignment is that the volume doesn't exist yet when we load the file and parse metadata leading to a sort of chicken and egg problem here. If we create the volume in PyDAGMC then we can assign the material easily enough.

One issue I can see with that is keeping track of which volume is the implicit complement when round-tripping the mesh to and from disk. Is marking the implicit complement part of the file spec?

pshriwise · 2025-11-05T05:21:15Z

test/test_dagnav.py

+    # Set the implicit complement material to the existing material of the graveyard
+    model.implicit_complement_material = 'Graveyard'
+    assert model.implicit_complement_material == 'Graveyard'
+    assert graveyard.material_group.name == 'mat:Graveyard_comp'
+    assert graveyard.material == 'Graveyard' # Should not change


As @gonuke mentioned above, I think there's some conflation of concepts here between the implicit complement and graveyard volume.

pshriwise · 2025-11-05T05:30:47Z

I'm not sure all the assumptions and conventions are robust to the rules for how the implicit complement and its material are used in practice. I wonder if we've every written down the expectations for how this information is used in codes? @pshriwise ?

Thanks for jogging my memory about this @gonuke as to the chicken and egg problem we run into when assigning the implicit complement material in OpenMC. Based on the website, this capability appears to be supported for MCNP, Tripoli4, and OpenMC.

Here are a couple of snippets from the page:

"Since the implicit complement doesn’t exist before running DAGMC and DAGMC can only recognize groups that contain an entity, the material for the implicit complement must be specified as if it were being specified for the graveyard volume."

"Since DAGMC only recognizes those groups that contain an entity, it is necessary to add a volume to this group, recognizing that this material will NOT be assigned to that volume"

This part of how the material assignment works is particularly confusing and understandably led to a lot of confusion in this PR as to it's connection with the graveyard volume (sorry @ahnaf-tahmid-chowdhury). As I understand it this is something that we could solve in DAGMC if we were to revisit the restriction around the group requiring a volume to be recognized.

As an alternative, we could add the capability here to create the implicit complement volume and assign its material like any other volume. Codes do tend to treat this volume specially, however, and I think we'd want to mark it as such as part of the DAGMC file spec were we to do this.

Plenty to discuss here it seems.

pshriwise · 2025-11-05T15:11:40Z

Plenty to discuss here it seems.

Linking in a related issue in the DAGMC repo here:

svalinn/DAGMC#937 (comment)

Feat: Add implicit complement and default material properties

0811f18

ahnaf-tahmid-chowdhury added 2 commits November 4, 2025 19:59

add more cov

dd8b1e7

test_change_implicit_complement_material

f9bea5e

gonuke requested changes Nov 5, 2025

View reviewed changes

pshriwise requested changes Nov 5, 2025

View reviewed changes

pshriwise mentioned this pull request Nov 5, 2025

Allow implicit complement material assignment on empty group/block svalinn/DAGMC#937

Open

Add implicit complement and default material properties #57

Are you sure you want to change the base?

Add implicit complement and default material properties #57

Uh oh!

Conversation

ahnaf-tahmid-chowdhury commented Nov 4, 2025

Uh oh!

codecov bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pshriwise left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pshriwise commented Nov 5, 2025

Uh oh!

pshriwise commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 4, 2025 •

edited

Loading