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

Consequences of bandmath as currently implemented and desire for a usecase tutorial #83

Open
gully opened this issue Mar 21, 2022 · 5 comments

Comments

@gully
Copy link
Member

gully commented Mar 21, 2022

The recent pull request #81 by @kfkaplan made us realize some unintended consequences of the "band math" changes to EchelleSpectrumList-like objects. See the conversation in that PR for the full breakdown.

I'm not sure one way or the other what we should do, since the status quo bandmath has some benefits and drawbacks. I'm opening this issue as a spot to have a conversation about it in a conspicuous place.

If we want to make some headway on deciding, I suggest we make an experimental tutorial with a working title "Unifying IGRINS H and K bands" or something like that, with whatever use case it is we want to support. We should get to the point where we achieve some complicated goal (without muler built in convenience functions) and then ask ourselves whether those patterns are going to be so frequent and uncontroversial that we should chisel them into muler's core.

In other words, let's include the a typical band math use case in that tutorial, doing it with and without the current bandmath implementation.

@kfkaplan
Copy link
Collaborator

My opinion is that the band math should be left as is, since it preserves that full functionality of the Spectrum1D class from specutils all the way through our EchelleSpectrum and EchelleSpectrumList classes, which are extensions of Spectrum1D.

Combining separate EchelleSpectrumList objects is a unique event that will probably just be confined to combining the IGRINS H & K bands thus should just be its own definition in utilities.py. I don't see it being used for much beyond that.

@gully
Copy link
Member Author

gully commented Mar 21, 2022

Thank you for your input Kyle!

Would you be open to making a demo tutorial showing how you use bandmath as is?

The muler paper is now under review at JOSS, and so it's conceivable a bird's eye view of the code will help us see from yet a different perspective.

@kfkaplan
Copy link
Collaborator

I'll see what I can do. Do you want something realistic (e.g. with sky subtraction, flux calibration, telluric correction)? or just a demo showing how band math works? I would have to add in some more example data if you wanted something realistic.

@gully
Copy link
Member Author

gully commented Mar 23, 2022

Simpler is better for tutorials, I think.

It's OK to add some more example data, especially since you are used to working with the rawer form of spec.fits files rather than spec_a0v.fits files, so showing that pattern may be useful to others.

Speaking of which, I can envision a successful tutorial that combines muler and its sibling package gollum. Have you used gollum yet? It's mostly focused on model stellar photosphere spectroscopy, so it may not be as relevant to your science depending on what you're up to these days.

I think you're right that allowing the user the freedom and flexibility to operate on the orders separately is useful and desirable. That makes more work for us the developers though, and so the decision should not be made too lightly, since it adds a bunch of corner-case scenarios to consider. On balance it seems like expanding what we enable in EchelleSpectrumList is good for the world and feasible to support.

@kfkaplan
Copy link
Collaborator

Actually yes I am using gollum to generate synthetic spectra for my standard stars. It so far has worked quite well. I could put together a tutorial for processing my Orion Bar data from Kaplan et al. (2017; 2019) which would include some band math since I do a subtraction of the OH residuals. I think I started to do this about a year ago so the data might already exist in the m data git repo. I will go see and upload any that might be needed and start throwing together a tutorial later today. It will be based on the work I am trying to do now, but with the published Orion Bar data substituted in.

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