search

GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
223 stars 105 forks source link

Fix chromatic save photons #1289

Closed rmjarvis closed 3 months ago

rmjarvis commented 3 months ago

Fixes a few issues with Chromatic objects' use of the save_photons=True option.

Specifically, issues #1271 and #1284.

rmjarvis commented 3 months ago

@jmeyers314 When working up the test suite for this, I had trouble with chromatic objects that use atRedshift. In particular ones that use the base class's implementation of atRedshift to just use ChromaticTransformation(obj, redshift=redshift). It's not even clear what this should do in the general case.

When the object is simply gal * sed, it's straightforward. Basically just apply atRedshift to the sed. And some slightly more complicated objects also work fine. But for instance what should atRedshift do to this object: galsim.Convolve(disk * disk_SED, optical_psf).atRedshift(1.2)? Only apply the redshift to disk_SED? Or also shift the wavelength dependence of the PSF? We have a test_atredshift in test_chromatic.py that enforces the latter, which seems weird to me. And I couldn't find an elegant way to implement the same thing for photon shooting. (That test uses FFT rendering.)

Anyway, my eventual takeaway was that maybe we should get rid of the whole option to call atRedshift on objects rather than just on SEDs. I think that would still cover all the cases anyone actually needs, and it would avoid the ambiguity about what convolutions at a redshift mean, since the user would be explicit about it. But do you know if that would get rid of some use case that needs this feature?

jmeyers314 commented 3 months ago

LGTM. I agree that we should only permit .atRedshift on SEDs. Redshifting the wavelength dependence of a PSF feels like an error.

rmjarvis commented 3 months ago

Thanks Josh. I'll do the atRedshift deprecation on a separate PR to keep things clean.