Skip to content
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.

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

vald broadening implementation #225

Merged
merged 34 commits into from
Dec 10, 2024

Conversation

jvshields
Copy link
Contributor

@jvshields jvshields commented Oct 28, 2024

This PR lets us use vald broadening parameters and interpolate them to the plasma, rather than calculate them ourselves. On top of the more direct comparison this lets us make to other codes, this also allows us to handle auto-ionization lines (see the line around 5172.4 angstroms below).

vald_broadening

@jvshields jvshields marked this pull request as draft October 28, 2024 21:04
@jvshields jvshields marked this pull request as ready for review November 8, 2024 21:00
@jvshields jvshields mentioned this pull request Dec 4, 2024
@@ -21,6 +23,10 @@
VACUUM_ELECTRIC_PERMITTIVITY = 1.0 / (4.0 * PI)
Copy link
Member

@Knights-Templars Knights-Templars Dec 9, 2024

Choose a reason for hiding this comment

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

Possibly this is not "vacuum electric permittivity". This looks like a constant not containing epsilon_0, mu_0, or c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this is weird and I can look into it, but it's not really part of the PR and is leftover from something I can't take credit for.

stardis/plasma/base.py Outdated Show resolved Hide resolved
@@ -414,8 +414,8 @@ def calculate(
linelist["level_energy_upper"] = ((linelist["e_up"].values * u.eV).cgs).value

# Radiation broadening parameter is approximated as the einstein A coefficient. Vald parameters are in log scale.
linelist["A_ul"] = 10 ** (linelist["rad"]) / (
4 * np.pi
linelist["A_ul"] = 10 ** (
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are 4 places calculating this coefficient in this PR, maybe it's not a problem but if there is a way to optimize it would be nice -- make it a properties of the linelist itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't quite know what you're suggesting here. Certainly the molecules and the elements need to be separate functions. The solution I think outside of this PR is just to always use the shortlist calculation, since they're functionally equivalent to the shortlist, but the shortlist needs less information.

But anyway, this is really not part of this PR. There's a bit of standardization slipped in, but this PR mostly doesn't touch the line integrated opacity calculations.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I see, so the linelist here is already a sliced shortlist of the original list? Sorry I was imaging the same linelist dataset and calculating the A_ul coff multiple times throughout different function. All good then.

@jvshields jvshields merged commit d0b5404 into tardis-sn:main Dec 10, 2024
5 checks passed
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.

4 participants