search

mperrin / poppy

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

return 1+0j if lens radius of curvature is infinite #245

Closed douglase closed 6 years ago

douglase commented 6 years ago

as discussed in this issue: https://github.com/pydata/numexpr/issues/293, NumExpr can (at least with some libraries) return NaN instead of 1+0j for complex valued expressions such as exp((-0.-0.j)/np.inf).

This checks for infinite radii of curvature and returns a lens phasor 1+0j, since exp(something/infinity) should always return 1.

mperrin commented 6 years ago

Hmm, this looks reasonable but the Travis test stalled out. I've kicked the job to try running again there and we will see whether that works or not.

Do you think it would be worth adding a unit test to verify this functionality?

douglase commented 6 years ago

it looks like Travis is dying on the fresnel tests, maybe we need to decrease the array size to use less memory again? They pass locally.

I have been thinking it might be worth adding a few unit tests to the accelerated math functions and this would follow under that umbrella.

mperrin commented 6 years ago

Yes that was my conclusion - running locally it was going up to 2.8 GB for the Python process and I believe Travis nodes are capped at 3 GB so I can easily see it running into the limit. I started modifying the test but seems like I will have to tweak the assertions to still get it to pass with the smaller size. I’ll probably get to this at some point today.

mperrin commented 6 years ago

Fixed the Fresnel test case to run even with lower sampling. This involved something I've long needed to do, improving the measure_fwhm function to better handle coarser sampling. And then being more careful about PSF centering when computing the FWHM. I'm running the updated tests on Travis now. Assuming that passes, then I can address the back log of PRs including this one.