-
Notifications
You must be signed in to change notification settings - Fork 105
Description
In #1775 I "innocently" added He as a new constants.IonProperty, as an alias to He4.
This significantly modified the step_flattop_bgb result. Looking deeper, it's because He is in the IDS and is now being picked up as an impurity, whereas before it wasn't.
-
@theo-brown , is having all 3 of Xe, Ar, He the desired impurity mix for this STEP case? If so then I can update the nc file in this PR. Note that the ~10% He made a very significant impact to the scenario, around ~50% different temperatures. It's probably good practice to use the optional 'expected_impurities' argument to avoid surprises
-
@MateoBell , thinking again, maybe it's a bit of a footgun to only parse impurities that are in
constants.ION_PROPERTIES_DICT.keys(), and ignore others . This can lead to surprises like we see here, that impurities that the user thinks should be valid, are ignored. Maybe if an unidentified impurity is found, we throw an error, and then instead of "expected_impurities" we can have an "excluded_impurities" so the user can choose to filter out impurities that maybe don't matter so much but TORAX can't handle (if these exist). WDYT?