Skip to content

Change expected_impurities to excluded_impurities in plasma_composition_from_IMAS #1806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
MateoBell wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Change expected_impurities to excluded_impurities in plasma_composition_from_IMAS #1806

MateoBell wants to merge 3 commits into from

Conversation

@MateoBell
Copy link
Collaborator

@MateoBell MateoBell commented Dec 3, 2025 •
edited
Loading

Closes #1789
Parse all ions by default to avoid ignoring ions without the user knowing.

Changes:

  • Rename expected_impurities to excluded_impurities arg. Now parse all ions in the IDS except those in excluded_impurities.
  • Add _validate_ids_ions helper function to validate that all parsed ions have recognized TORAX symbols and raise informative error if not
  • Updated tests to match new behavior

@MateoBell MateoBell added the enhancement New feature or request label Dec 3, 2025
@MateoBell MateoBell changed the title Change expected_impurities to excluded_impurities Change expected_impurities to excluded_impurities in plasma_composition_from_IMAS Dec 4, 2025
@MateoBell MateoBell marked this pull request as ready for review December 4, 2025 10:47
@MateoBell MateoBell added the ready for review Add to PRs when they are ready for review unsure of who to assign as reviewer label Dec 4, 2025
This was referenced Dec 7, 2025
Closed
Open
@jcitrin
Copy link
Collaborator

jcitrin commented Dec 7, 2025

@Nush395 could you also review this one please?

Nush395
Nush395 reviewed Dec 8, 2025
View reviewed changes
@MateoBell MateoBell requested a review from Nush395 December 10, 2025 12:54
@MateoBell MateoBell force-pushed the update_imas_plasma_composition_parsing branch from ebae3bf to e9458b8 Compare December 11, 2025 12:36
@MateoBell MateoBell force-pushed the update_imas_plasma_composition_parsing branch 2 times, most recently from e94622d to d61a1c8 Compare December 20, 2025 10:26
Nush395
Nush395 reviewed Dec 20, 2025
View reviewed changes
@MateoBell
Change expected_impurities to excluded_impurities
c4a358c 
Closes IMAS plasma composition surprising behaviour #1789
Parse all ions by default to avoid ignoring ions without the user knowing

Update imas sim_test example

correctly parse ions

Slight change of error message

Add comment for str(ion) cast
@MateoBell
Change default main_ion_symbols to None
0228f57
@MateoBell
separate function to validate main_ions presence
f2181d3
@MateoBell MateoBell force-pushed the update_imas_plasma_composition_parsing branch from d61a1c8 to f2181d3 Compare January 5, 2026 14:56
@MateoBell MateoBell mentioned this pull request Jan 12, 2026
Open
@MateoBell
Copy link
Collaborator Author

MateoBell commented Jan 13, 2026 •
edited
Loading

@Nush395 : Should these new functions go into validation.py now ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nush395 Nush395 Nush395 left review comments

@jcitrin jcitrin jcitrin approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request ready for review Add to PRs when they are ready for review unsure of who to assign as reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

IMAS plasma composition surprising behaviour

3 participants

@MateoBell @jcitrin @Nush395