Skip to content

Conversation

shimwell
Copy link
Member

Description

This PR changes the behaviour of material.add_nuclide so that if a nuclide with the same name and percentage type exists in the material then the new nuclide will be combined with the existing one.

If the nuclide exists in the material but has a different percentage type then a warning message will be printed to the user.

I think this PR puts us in better place than currently as it solves the issue of adding like nuclides in some cases and warns the user in other cases. Previously the nuclides were silently duplicated.

As discussed on the forum
https://openmc.discourse.group/t/question-about-percent-type-argument-of-the-new-method-add-elements-from-formula-introduced-in-version-0-12/702/5

Fixes # (issue)

Checklist

  • I have performed a self-review of my own code
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@makeclean
Copy link
Contributor

Not suggesting that this should be a blocker, but I've heard a number of times from analysts they often like to see repeated nuclides when they come from different mixtures, i.e. keeping the contributions from different nucldies printed in blocks by mixtures, repeating nuclides as needed. I can see it both ways, you have a clear record in the python file which made your problem, so thats tracable, alhtough you might keep the xml file left behind. So might it be worth haveing a flag which gives the option to squash as an argument to that function, which defaults to true?

@shimwell
Copy link
Member Author

shimwell commented Sep 11, 2025

analysts they often like to see repeated nuclides when they come from different mixtures, i.e. keeping the contributions from different nucldies printed in blocks by mixtures, repeating nuclides as needed.

ok fair enough,

optional squashing (default true) when exporting to xml sounds like a more suitable solution then

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