Skip to content

Fix krypton calculation - #31

Merged
tbody-cfs merged 25 commits into
mainfrom
krypton_fix
Apr 27, 2026
Merged

tbody-cfs merged 25 commits into
mainfrom
krypton_fix

Conversation

@tbody-cfs

@tbody-cfs tbody-cfs commented Apr 21, 2026

Copy link
Copy Markdown
Collaborator

This PR addresses an issue where the radiated power $L_z$ was returning zero for Krypton.

The Issue

The atomic data files from OpenADAS for Krypton have slightly different electron temperatures at a single position

print(adas_datasets["effective_recombination"].dim_electron_temp.values[32])
print(adas_datasets["line_emission_from_excitation"].dim_electron_temp.values[32])

returns

7170.839465426523
7170.674352646886

When merging xr.DataArrays into an xr.Dataset, a silent xr.merge(..., join="outer") is used. This fills NaNs where data is "missing", and this goes on to corrupt the entire Lz calculation.

To fix this, we now interpolate before combining the rate coefficients into a single dataset, and then use xr.merge(..., join="exact") which will raise an error if the axes are not perfectly aligned.

In addition, this model slightly changes the plotting, to make it a bit easier to see when Mavrin data isn't defined.
argon
krypton

Github Actions are extended to run poetry run radas for all species on new releases, or if manually triggered by a user.

@tbody-cfs
tbody-cfs requested a review from MishaVeldhoen April 22, 2026 00:01

@MishaVeldhoen MishaVeldhoen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Tom, good fixes overall! My main worry is about the silent grid extrapolation due to clipping, and publishing of OpenADAS data.

I also find (more generally) some parts of the code are a bit hard to read because the JSON config has a compact structure, the details of which leak into the code itself. E.g., filename can be an int, or a list, which reads unnaturally. Could be fixed in a next PR where we fix up config parsing with dataclasses or pydantic models

Comment thread .github/workflows/workflow_actions.yml Outdated
Comment thread .github/workflows/workflow_actions.yml
Comment thread .github/workflows/workflow_actions.yml
Comment thread .github/workflows/workflow_actions.yml
Comment thread .github/workflows/workflow_actions.yml Outdated
Comment thread radas/cli.py Outdated
Comment thread radas/config.yaml
Comment thread radas/interpolate_rates.py
Comment thread radas/read_rate_coeffs.py

@MishaVeldhoen MishaVeldhoen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tbody-cfs
tbody-cfs merged commit c0d597a into main Apr 27, 2026
6 checks passed
@tbody-cfs
tbody-cfs deleted the krypton_fix branch April 27, 2026 17: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