Skip to content

Raise on duplicate material names in convert_to_multigroup - #108

Open
jon-proximafusion wants to merge 2 commits into
developfrom
fix-convert-mgxs-duplicate-material-names
Open

Raise on duplicate material names in convert_to_multigroup#108
jon-proximafusion wants to merge 2 commits into
developfrom
fix-convert-mgxs-duplicate-material-names

Conversation

@jon-proximafusion

@jon-proximafusion jon-proximafusion commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Model.convert_to_multigroup() generates one MGXS per material, keyed by material name. Two distinct materials that share a name would be silently written as a single cross section — one material's data overwrites the other's, dropping a material. This is easy to hit because Material.clone() (used by Model.differentiate_mats()) preserves the original name, and because names are sanitised to alphanumeric + underscore, which can make otherwise-distinct names collide.

Conversion now raises a ValueError instead of silently producing an incorrect library; material names are left unchanged. The per-material granularity is also noted in the convert_to_multigroup docstring.

Tests

Added a unit test asserting that two distinct materials sharing a name raise ValueError.

@jon-proximafusion
jon-proximafusion force-pushed the fix-convert-mgxs-duplicate-material-names branch from 30b2ad2 to 0afedb4 Compare June 25, 2026 13:58
@jon-proximafusion jon-proximafusion changed the title Fix silent material collapse in convert_to_multigroup Warn about ambiguous and averaged materials in convert_to_multigroup Jun 25, 2026
@jon-proximafusion
jon-proximafusion force-pushed the fix-convert-mgxs-duplicate-material-names branch from 0afedb4 to d5b560a Compare June 25, 2026 14:17
@jon-proximafusion jon-proximafusion changed the title Warn about ambiguous and averaged materials in convert_to_multigroup Raise on duplicate material names in convert_to_multigroup Jun 25, 2026
@jon-proximafusion
jon-proximafusion force-pushed the fix-convert-mgxs-duplicate-material-names branch 2 times, most recently from a3fb34d to 5c29764 Compare June 25, 2026 14:30
Comment thread openmc/model/model.py Outdated
@jon-proximafusion
jon-proximafusion force-pushed the fix-convert-mgxs-duplicate-material-names branch from 5c29764 to abb819a Compare June 25, 2026 14:32
convert_to_multigroup generates one MGXS per material, keyed by material
name. Two distinct materials that share a name (for example the same-named
clones from Model.differentiate_mats(), since Material.clone() preserves
the name) cannot be written as separate cross sections -- one would
silently overwrite the other, dropping a material's data. Names are also
sanitised to alphanumeric plus underscore, which can make otherwise
distinct names collide.

Raise a ValueError instead of silently producing an incorrect library,
leaving user-assigned names unchanged.
@jon-proximafusion
jon-proximafusion force-pushed the fix-convert-mgxs-duplicate-material-names branch from abb819a to 5a5f365 Compare June 25, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants