GalSim-developers / GalSim

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

Persist pupil samples uv in photon array #1147

Closed jmeyers314 closed 2 years ago

jmeyers314 commented 2 years ago

Abuses the PhotonArray.dxdz and .dydz attributes to hold onto pupil samples generated by PhaseScreenPsf._shoot.

I realized along the way that we may wish/need to propagate these values forwards through photon array convolutions, so the rule is if exactly 1 convolutant has .dxdz or .dydz, then the result will just get the same values. If more than one convolutant has these attributes, then an exception is raised.

I went ahead and also implemented similar logic for wavelengths, except in this case, GalSim already had a case where two photon arrays with populated wavelength arrays would be convolved; in ChromaticObject.applyTo. Here, it's actually innocuous, since the two photon arrays will have the same wavelength arrays by construction. So to allow this code to continue as is, I also decided to permit convolution of two photon arrays both with populated wavelength arrays as long as those arrays are identical (via the python is operator).

jmeyers314 commented 2 years ago

I'm wondering now if it'd be better design to just add new pupil_u, pupil_v attributes to PhotonArray without reusing the dxdz, dydz storage. Both cases can be allocated on-the-fly, and we can always reclaim the storage when we're done by explicitly setting these back to None. This way we don't have to handle collisions in meaning.

jmeyers314 commented 2 years ago

(And in fact, we might not even need to expose pupil_u, pupil_v to the c++ layer.)

rmjarvis commented 2 years ago

That also sounds fine to me. If any C++ function down the line wants them, we can add that exposure then.

jmeyers314 commented 2 years ago

Added pupil_u and pupil_v as PhotonArray attrs. This is ready again Mike.

rmjarvis commented 2 years ago

LGTM. I like this UI better.