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

Remove variants? #35

Open
brryan opened this issue Oct 19, 2022 · 3 comments
Open

Remove variants? #35

brryan opened this issue Oct 19, 2022 · 3 comments

Comments

@brryan
Copy link
Collaborator

brryan commented Oct 19, 2022

We use a variant just for units vs. not units, especially in scattering opacities. It would be nice to find a way around this, something like @Yurlungur 's

I was imagining that in Phoebus, we just always have units. And non-cgs units just sets the conversion factor to 1? Is there a reason that wouldn't work? We could even do something like

typedef MeanS = MeanSWithUnits<MeanSGray>;

or something silly like that

@brryan brryan mentioned this issue Oct 19, 2022
@brryan
Copy link
Collaborator Author

brryan commented Nov 1, 2022

Because of the issue with NonCGS opacities not being acceptable to the MeanOpacity constructor, maybe MeanOpacity should be subsumed into the regular Opacity object, removing some variants.

Also, it might be nice to be able to enroll non-standard opacities as an end user. E.g. toy opacity models that don't necessarily belong in this repository. This would probably require some kind of preprocessor-like scripting but might not be too bad.

@Yurlungur
Copy link
Collaborator

I like keeping the toy models here---I think that's useful. I agree we should reduce the number of variants somehow.

@brryan
Copy link
Collaborator Author

brryan commented Nov 2, 2022

Also worth revisiting how we handle rho -> n conversions, especially in scattering opacities. It may be worth e.g. providing X, Y, and Z (hydrogen, helium, other) mass fractions through *lambda, possibly also with some notion of ionization state? Once we get to ionization state, though, opacities may just be tabulated anyway.

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

No branches or pull requests

2 participants