Include prism bandpasses in the roman module

[arunkannawadi]

Addresses #1018

[arunkannawadi]

There's a lot of chatter about setuptools removing the test command. How do we want to fix the breakage?

https://setuptools.pypa.io/en/stable/history.html

[rmjarvis]

Eugh. I guess pin setuptools to <72 for now. We use it to run the C++ tests. I guess we'll need to come up with some other builder/runner for them.

[rmjarvis]

BTW, are you planning to actually do something to implement prism observations? This PR doesn't do anything except allow those bands to be selected. I think that's a bad idea if we don't also do something to make them simulate the prism dispersion physics.

[arunkannawadi]

I figured that somebody would be confused because the changes in the PR doesn't do justice to the title :-)

Troxel and I have generated prism images along with Roman SN PIT with these changes here, some in skyCatalogs here using the dispersive photon operator here. All of these come together in the roman_imsim package here.

I'd still like to have this merged by v2.6 since someone at STScI is going to take this over from us and I'd like them to be able to use GalSim+skyCatalogs without needing any modifications. Apart from having a similar photon operator here in the unit tests, do you have anything in mind? I am not sure if this would be meaningful at all.

[rmjarvis]

I feel like some kind of sanity check is warranted here. I'm worried that someone who isn't using the roman_imsim code will just try to make an image with one of these as the filter, and then be upset that the image doesn't actually have any dispersion. I don't know what the best place for it is though.

Maybe just add a logger warning along the lines of "Warning: selecting a dispersive bandpass. This should be accompanied by a suitable photon operator (e.g. from roman_imsim) to actually implement the dispersive effect. This compatibility is not checked."

@rmandelb @matroxel What do you think?

[matroxel]

We need to at least get the right thermal backgrounds for these bandpasses and an implementation of the zodiacal light that works before merging into main I'd think?

A warning would be good, and we should probably include an example dispersion photon op and some updated documentation in demo13 or a new demo?

[arunkannawadi]

Do we have the thermal backgrounds from the project? For other short wavelength bands, they are unavailable and set to 0 here, so I followed the same. We have nominal zodical backgrounds through those bandpasses here although they may not be precise enough for prism.

Warning + demo with dispersion photon operator sounds good. If we have to switch the backgrounds, then I'd suggest we keep the issue open but merge this PR with warning+demo.

[matroxel]

The blue bands round to zero at some decimal place. The wide prism/grism bandpasses won't. This is something we'll have to ask about, its not on the Roman page that I found.

[matroxel]

I guess we should be properly accounting for it vs wavelength like we will for the zodiacal light for the W filter and prism/grism. W filter is also not well represented right now.

[arunkannawadi]

For some context, Roman SN PIT folks just want to be able to test their spectral extraction code from a reasonable-looking image, not necessarily realistic. Depending on when v2.6 is aimed for, getting realistic backgrounds may be out of scope. For this initial PR, my minimal goal is to be able to get the bandpasses by calling galsim.roman.getBandpasses(), which is not possible at present.

[arunkannawadi]

I changed the PR title to reflect the code changes.

For the warnings, I could think of two ways - either include an optional argument to getBandpasses that will decide whether to fetch non-imaging bands and warn the user if they are fetched or modify the __getitem__ method for the bandpass_dict that will warn if the keys corresponding to non-imaging bands are passed. I kinda like the second one, but I don't know if this would annoy users who might be used to just iterating over all the bands.

[rmjarvis]

Here's an idea, which might be better anyway. Add an option to getBandpasses like include_nonimaging_bands=True, which would include the Prism and Grism, which defaults to False. And maybe then it would be sufficient to just put a warning in the doc string about how that isn't sufficient to get prism physics and where the user might go to get it. At least then, it requires the user to be more intentional about selecting those bands, so maybe they would see the documentation about it.

[rmjarvis]

I'm happy with this now. @matroxel or @rmandelb did either of you want to take a look?

[rmjarvis]

@arunkannawadi Looks like you might need to rebase it to the current main. GitHub is complaining.