Allow for all (non-imaging) Roman bands internally#1347
Conversation
galsim/roman/roman_config.py
Outdated
| depr('W149', 2.5, 'W146', 'Note: this is to match current Roman filter naming schemes') | ||
| name = 'W146' | ||
| bandpass = getBandpasses()[name] | ||
| bandpass = getBandpasses(include_all_bands=True)[name] |
There was a problem hiding this comment.
I think here and in _get_single_PSF below, we could use
include_all_bands = (name in galsim.roman.non_imaging_bands)
to be a little more efficient.
Honestly, we should probably have added an option in getBandpasses to optionally only read in a single band by name. That would be even more efficient for these cases where we will just throw away all but one of the bands immediately. Up to you whether you want to add that.
There was a problem hiding this comment.
Good idea, Mike. I didn't quite realize that getting all the bands was non-trivial. Implemented.
arunkannawadi
force-pushed
the
all_roman_bps
branch
from
a8045f3 to
021ccd4
Compare
By default, we don't want to return the non-imaging bands when a user asks for it by calling galsim.roman.getBandpasses(). But, in the internal calls, it should get all the bands so that if the user asks for a non-imaging bandpass through other methods, they should be able to get them.
The index based lookup for sky backgrounds is finicky when requesting only a subset of bandpasses. This could be done by looking up the index of the bandname, but looking up by column name is more robust and has the advantage of documenting the shared data file better.
arunkannawadi
force-pushed
the
all_roman_bps
branch
from
021ccd4 to
3c83a78
Compare
getBandpass and getBandpasses stay in sync.
rmjarvis
left a comment
rmjarvis
left a comment
There was a problem hiding this comment.
LGTM. Thanks Arun. And nice improvement in the implementation using genfromtxt.
|
Thanks, Mike. Let me add a test with the W band, because I'm not 100% sure that that works with |
|
OK. No problem. I'll wait till you have that then. |
|
I made the additional changes I wanted to make. You should be good to merge this after the checks are all green. |
2 participants
By default, we don't want to return the non-imaging bands when a user asks for it by calling galsim.roman.getBandpasses(). But, in the internal calls, it should get all the bands so that if the user asks for a non-imaging bandpass through other methods, they should be able to get them.
This was the only change I had to make in GalSim to get simulations with prism/grism working (ref: #1018)