search

GalSim-developers / GalSim-Euclid-Like

Helper functions to generate simulations of Euclid-Like images.
Other
1 stars 0 forks source link

bandpass work #87

Closed rmandelb closed 1 month ago

rmandelb commented 1 month ago

This PR has two small changes:

  1. Documentation of the bandpass normalization
  2. Moving the VIS bandpass limit setting into euclidlike, which addresses #80
rmandelb commented 1 month ago

Not ready for review - a test failed and I'll have to diagnose

FedericoBerlfein commented 1 month ago

I want to raise a known issue about these cuts. If you plot the VIS throughput for the entire bandpass, you can see that the red_limit cut is missing a chunk of the throughput at higher wavelengths. The plot below shows this (vertical dashed lines show 540 and 910 cuts).

Screenshot 2024-09-26 at 3 29 00 PM

The wavelength space of the provided PSF's is 534.3-910.7, which is why we use these particular cuts for drawing the PSF. The filter throughout at current two blue/red limit values we are cutting the bandpass at are f(540) = 0.11 and f(910) = 0.38. For the purposes of calculating the flux (at the catalog level), do we want to use the full bandpass or the one with the blue/red limit cuts in the PSF? For a vega-like SED, the cut in the red limit causes a ~1.8% reduction in the flux, while the blue limit cut is only ~0.005%.

rmandelb commented 1 month ago

I think we should keep it simple: use the same red/blue limits as needed for the PSF when calculating catalog-level fluxes. We can simply document the percent-level effects and leave it at that.

However, if there is a preference to omit those red/blue limits when calculating fluxes, I guess I could make a keyword argument to enable this. Thoughts?

FedericoBerlfein commented 1 month ago

I think we should keep it simple: use the same red/blue limits as needed for the PSF when calculating catalog-level fluxes. We can simply document the percent-level effects and leave it at that.

However, if there is a preference to omit those red/blue limits when calculating fluxes, I guess I could make a keyword argument to enable this. Thoughts?

I think that would be helpful if the user wants to use the full bandpass.

rmandelb commented 1 month ago

This is now ready for a review - tests pass etc. (I hadn't initially realized that the hardcoded bandpass limits were also being applied in the tests, so it was re-applying them, which breaks things.)

rmandelb commented 1 month ago

I enabled the option to use the full bandpass, and made a test for that.