Open
Conversation
Extract parse_cell_material_xml() and parse_cell_temperature_xml() from CSGCell::CSGCell() into free functions in cell.h/cell.cpp so that DAGUniverse can reuse the same logic when parsing <cell> override sub-elements. Both handle void/material-id conversion, empty-array validation, and non-negative temperature validation. Each call site retains its own higher-level policy (fill/material exclusivity in CSGCell, forbidden-attribute guards and duplicate detection in DAGUniverse). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the TypeError stub in DAGMCCell.from_xml_element with a proper implementation. Validates DAGMC-unsupported attributes (region, fill, universe, density, translation, rotation, volume) and required material, then delegates to Cell.from_xml_element for common id/name/material/ temperature parsing via a no-op universe stub. DAGMCUniverse._parse_cell_ overrides is simplified to loop over <cell> elements and call DAGMCCell.from_xml_element, removing all duplicated parsing logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Density overrides: - Add parse_cell_density_xml() free function (cell.h/cell.cpp), used by both CSGCell::CSGCell (replacing inline parsing) and DAGUniverse XML constructor - Add DensityOverrides type alias and thread it through initialize() and init_geometry() in DAGUniverse; apply density_mult_ in init_geometry() using the same deferred-multiplier convention as CSG cells - Remove density from the forbidden-attribute block in the DAGMC XML constructor and from DAGMCCell validation Volume overrides: - Volume is Python-only (not stored in C++ Cell); no C++ changes needed - Remove volume from forbidden-attribute checks in DAGMCCell - Add a Notes section to DAGMCCell docstring warning that manually specifying volume may be inconsistent with DAGMC's triangulated-surface geometry, which can support exact volume computation translation and rotation remain unsupported and are rejected explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers: material/temperature/density/volume parsing, forbidden attribute rejection (region, fill, universe, translation, rotation), and missing material error. Added to test_cell.py alongside existing Cell XML tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hema Both Python and C++ now accept the old <material_overrides> format with a DeprecationWarning/warning() instead of a hard error, converting each <cell_override> into an equivalent DAGMCCell or MaterialOverrides entry. - Python: replace the ValueError in DAGMCUniverse.from_xml_element with a DeprecationWarning; add _parse_legacy_material_overrides() which builds DAGMCCell objects from <cell_override id> / <material_ids> text - C++: replace the fatal_error in DAGUniverse XML constructor with a warning() + conversion loop into material_overrides map - Both sides raise an error if <material_overrides> and <cell> appear together on the same <dagmc_universe> - Replace test_dagmc_xml_reject_legacy_material_overrides with six new tests covering single-material, distribmat, void, both-raises, deprecation-warning, and round-trip-to-new-format scenarios Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes DAGMCUniverse.material_overrides (getter/setter), replace_material_assignment, _get_cell_material_overrides, and the associated _material_overrides internal attribute. Cell material assignments are now managed directly via DAGMCCell objects. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…stub DAGMCCell.from_xml_element now accepts an optional universe parameter. When called from _parse_cell_overrides the actual DAGMCUniverse is passed, so Cell.from_xml_element registers the cell directly rather than requiring a separate add_cell call. Remove tests for the deleted material_overrides setter, replace_material_assignment, and stale error message patterns. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the deprecated <material_overrides> documentation with the new <cell> sub-element spec covering id, name, material, temperature, density, and volume. Include unsupported attribute list and a deprecation note for the old format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both the getter and setter raise AttributeError with a message directing users to the DAGMCCell-based API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
universe is now a required parameter since from_xml_element is always called from a DAGMCUniverse context. The test uses a clearly named placeholder_univ with a comment explaining the direct call is only for testing purposes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@bam241 I'd especially like to hear your thoughts on this one if you have the time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This takes a different approach to material overrides by reusing the current XML cell specification. The goal is to provide better support for not only material overrides but other cell properties as well (e.g. temperatures, densities). Later, with support for lattice/universe fills of DAGMC cells additional properties like translation and rotation will become useful as well.
Things to consider:
Claude's PR Description
Summary
This PR replaces the ad-hoc
<material_overrides>XML schema for DAGMC universes with ageneralized
<cell>sub-element approach that reuses OpenMC's standard cell XML schema.The new format supports material, temperature, density, and volume overrides on a per-cell
basis, with full parity between the Python and C++ interfaces.
Motivation
The previous
<material_overrides>schema was limited to material assignment only and useda bespoke XML structure not shared with the rest of OpenMC's geometry input. This PR:
<cell>XML schemamaterial_overridesproperty,replace_material_assignmentmethod, and related internal state from
DAGMCUniverse<cell>Sub-element SpecificationA
<cell>element nested inside a<dagmc_universe>overrides properties of the DAGMCvolume with the matching
id.Supported Attributes
idnamematerialvoidfor vacuum. Multiple IDs specify a distribmat fill. May also be a<material>child element.temperature<temperature>child element.density<density>child element.volumeUnsupported Attributes
The following standard
<cell>attributes are not valid inside<dagmc_universe>and willraise an error:
regionfilluniversetranslationrotationExample
Backward Compatibility
The old
<material_overrides>format is deprecated but still accepted in both Pythonand C++. A
DeprecationWarning(Python) orwarning()(C++) is emitted, and the overridesare converted to the new
<cell>representation at parse time. It is an error to mix bothformats on the same universe.
Implementation Details
C++ (
src/cell.cpp,include/openmc/cell.h)Three free functions are added to centralize
<cell>XML parsing, shared by bothCSGCell::CSGCellandDAGUniverse:parse_cell_material_xml(node, cell_id)— parsesmaterialattribute/element, handles"void"parse_cell_temperature_xml(node, cell_id)— parsestemperature, validates ≥ 0parse_cell_density_xml(node, cell_id)— parsesdensity, validates > 0DAGUniverseuses intermediate maps (MaterialOverrides,TemperatureOverrides,DensityOverrides) rather than applying values directly, because DAGMCCellobjects donot exist until the
.h5mfile is loaded duringinit_geometry().Python (
openmc/dagmc.py)DAGMCCell.from_xml_elementis added as a classmethod that delegates toCell.from_xml_elementviasuper(). It accepts an optionaluniverseparameter; whencalled from
_parse_cell_overridesthe owningDAGMCUniverseis passed directly so thecell is registered in one step without a separate
add_cellcall.DAGMCUniverse._parse_legacy_material_overrideshandles the deprecated XML format.material_overridesproperty is retained but both the getter and setter now raiseAttributeErrorwith a message directing users to theDAGMCCell-based API.replace_material_assignmentmethod,_get_cell_material_overrideshelper, andthe
_material_overridesinternal attribute are removed entirely.Tests
tests/unit_tests/test_cell.py—test_dagmccell_from_xml_element: round-trip ofmaterial, temperature, density, and volume; rejection of forbidden attributes; missing
material error.
tests/unit_tests/dagmc/test_model.py:test_dagmc_xml_legacy_single_material_compat— old format, single materialtest_dagmc_xml_legacy_distribmat_compat— old format, multiple material IDstest_dagmc_xml_legacy_void_compat— old format,voidmaterialtest_dagmc_xml_legacy_both_raises— error when both formats present on same universetest_dagmc_xml_legacy_deprecation_warning— confirmsDeprecationWarningis emittedtest_dagmc_xml_legacy_roundtrip— old format parsed → new<cell>format emittedChecklist