GalSim-developers / GalSim

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

Save photons from the component objects of a ChromaticSum #1285

Closed welucas2 closed 3 months ago

welucas2 commented 4 months ago

This commit addresses #1284 by checking whether image already has a photons attribute at the end of GSObject.drawImage. If not, it simply assigns it the newly created photon array as previously. If it does exist, a new array with the correct length is created, the extant image.photons copied in at the beginning and the new photons after, and the new array assigned to image.photons.

This fix corrects the example in #1284 and also fixes previously incorrect output from the imSim photon pooling pipeline noted in LSSTDESC/imSim#424.

rmjarvis commented 4 months ago

@welucas2 You'll need to rebase this to the current main to get the fix to CI in #1288.

welucas2 commented 4 months ago

Thanks for the heads-up @rmjarvis, just rebased.

rmjarvis commented 4 months ago

I don't think this is the right fix for this. If you repeatedly draw things and save photons each time, we won't want more photons appended each time. We'd want them overwritten. I think the fix needs to happen in ChromaticSum itself, not in GSObject. But rather than have you delve into the guts of the code, I can tackle this along with #1271.

welucas2 commented 4 months ago

I'm certainly not familiar with all the use cases so I'll take that as the correct approach! I guess the imSim photon pooling is only concerned that the photon array coming back out again from ChromaticSum contains the complete set of photons drawn for the object as a whole. If doing it in ChromaticSum, maybe the full-sized array can be created and then photons coming back from drawImage can be copied in analogously to added_flux..?

rmjarvis commented 3 months ago

See #1289 instead.