LSSTDESC / imSim

GalSim based Rubin Observatory image simulation package
https://lsstdesc.org/imSim
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

Fix flux treatment of FFT renders to not double the Poisson noise #420

Closed rmjarvis closed 1 year ago

rmjarvis commented 1 year ago

cf. Issue #419

  1. Switch to using the nominal flux for FFT renders, since we then add Poisson noise to the stamp image, which already does the work of Poisson-ifying the realized flux.
  2. Switch the flux value used for all decisions (phot vs fft, faint vs not) to use the nominal flux, rather than the realized flux.
  3. Changed the meaning of realized flux in the output file to be the actual number of photons that landed on the image, rather than the number of photons shot. This way vignetting is consistently included in this definition regardless of whether the object used phot or fft.
  4. Added phot_flux and fft_flux values in the output file. Only one of these is non-zero, and it is the flux used for each rendering type. So phot_flux is what used to be called realized_flux for photon shooting. And fft_flux is what used to be called realized_flux for fft renders. These each have slightly different meanings, but they are well-defined for each method.
  5. Added tests of all these flux values in the output catalog.
rmjarvis commented 1 year ago

FYI, I also fixed the CI memory crash problem. The issue was with diffraction_fft being turned on in test_process_info. It takes about 6 GB of memory apparently. So this might explain some of the memory spikes in Jim's memory profiles for some objects. Anyway, I turned it off in this test, but something we should keep in mind.

cwwalter commented 1 year ago

Thanks Mike!

jchiang87 commented 1 year ago

Should we go ahead and merge this? Everything looks good to me.

rmjarvis commented 1 year ago

Sure.