mperrin / poppy

Physical Optics Propagation in Python
BSD 3-Clause "New" or "Revised" License
173 stars 40 forks source link

Kolmogorov turbulent phase screens #249

Closed DaPhil closed 3 years ago

DaPhil commented 6 years ago

Hi. I implemented and tested turbulent wavefront errors within Kolmogorov theory. Added a notebook that describes the basics. Hope this is of use to anyone.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.1%) to 72.64% when pulling 5c312c3123651309e1adf8f31d003385613047d9 on DaPhil:kolmogorov into a276feb4abf4e301912f355d567913a3b7222bae on mperrin:master.

mperrin commented 6 years ago

A pleasant surprise to see this PR! As you saw, I had started some code for this but never had time or priority to get it working properly. This contribution is definitely welcomed.

I see that you made it so the WFE is recalculated automatically with a different random instance for each call to get_opd. That's one thing I had thought about doing differently, to support polychromatic calculations in which one would want different wavelengths to all encounter the same phase screens. Perhaps there could be an option to freeze the OPD to a fixed example of the Kolmogorov phase screen to support cases like that. I'll look through the code to see if I have any other comments or suggestions. But I wanted to ask what you thought of this idea.

Two of the CI system tests cases failed. One is just the same issue with a file encoding as before. Easy fix now, same as before. The other was a failure to build the documentation; this confuses me since it seems totally unrelated to the code changes. I don't know what to make of that one.

DaPhil commented 6 years ago

Based on this issue but without any real knowledge of what I am speaking of, it seems that the astropy helpers need to be updated. However, I committed a changed astropy_helpers which had a changed Subproject commit line. As I said, I have no idea what this is but maybe this info helps?

DaPhil commented 6 years ago

I see that you made it so the WFE is recalculated automatically with a different random instance for each call to get_opd. That's one thing I had thought about doing differently, to support polychromatic calculations in which one would want different wavelengths to all encounter the same phase screens. Perhaps there could be an option to freeze the OPD to a fixed example of the Kolmogorov phase screen to support cases like that. I'll look through the code to see if I have any other comments or suggestions. But I wanted to ask what you thought of this idea.

You could just save the first seed and use it to initialize the phase screens. However, you would need to recalculate all phase screens for all wavelengths which is redundant computing. One could also just save the phase screens for the first wavelength and then use them appropriately for the others (although I am not sure about it, does the pixelscale change with wavelength during propagation? The screens depend on the pixelscale!). But I am not familiar with how you deal with polychromatic waves. If you have them somehow computed in parallel, then at least the second method would not work (and the first still requires a recomputation of all screens although once would be enough).

mperrin commented 3 years ago

Closed here in this deprecated repo - see instead https://github.com/spacetelescope/poppy/pull/437 in the spacetelescope repo (which was merged already)