Skip to content

Conversation

@MateoBell
Copy link
Collaborator

@MateoBell MateoBell commented Dec 3, 2025

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
@jcitrin
Copy link
Collaborator

jcitrin commented Dec 7, 2025

@Nush395 could you also review this one please?

@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
@MateoBell MateoBell force-pushed the update_imas_plasma_composition_parsing branch from d61a1c8 to f2181d3 Compare January 5, 2026 14:56
@MateoBell
Copy link
Collaborator Author

MateoBell commented Jan 13, 2026

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

@Nush395
Copy link
Collaborator

Nush395 commented Jan 18, 2026

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

Sorry for the slow reply. I think moving into validation.py would make sense.

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

Change default main_ion_symbols  to None

separate function to validate main_ions presence
@MateoBell MateoBell force-pushed the update_imas_plasma_composition_parsing branch from f2181d3 to f7ff91a Compare January 19, 2026 13:03
@MateoBell
Copy link
Collaborator Author

Ok I moved the files and add small unit tests now they are in separate module ! Also rebased onto main and semi-squashed it. Let me know if there is still any change to make

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

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

Development

Successfully merging this pull request may close these issues.

IMAS plasma composition surprising behaviour

3 participants