Closed jmeyers314 closed 3 years ago
This sounds very reasonable. My one quick API push back would be to not make the function start with an underscore. There's no reason for this to be a "private" method. It should be a normal classmethod in good standing for users to use and expect good documentation and some version stability, IMO. So I think it should just be named fromArrays
.
My thought for making this "private" was that I didn't want to necessarily spend cycles checking if the arrays are all the same length, dtype=float, contiguous, etc. I think a public interface probably should check those things. One solution would be to implement both, of course.
Sure. Could do our normal thing of having the public version do the checks and then call the private version that doesn't. And I kind of consider these "leading underscore" versions with otherwise the same name to be part of the official API. As opposed to functions that start with an underscore without a corresponding public function, which we use as implementation details.
I've been thinking a bit more about how we might batch photons together to efficiently raytrace them using
batoid
. I think it'd be useful to be able to construct aPhotonArray
from (portions of) previously allocated numpy arrays. E.g.,I.e., with
PhotonArray._fromArrays
one could perform the steps indrawImage
, using multiprocessing to fill in independent objects photons "at the entrance pupil", then use batoid to trace them all simultaneously, and then finish with accumulate. In fact, I think most of our photonOps, and even the atmospheric kicks, could be done simultaneously, not just batoid. (Though DCR probably doesn't work, since that requires a local_wcs which will be different for different objects). I'm not sure how to integrate this intodrawImage()
itself (probably requires some kind of delayed-execution, which would be tricky), but this at least provides a hook to drawing things "manually".