Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 3, 2018

Conversation

douglase
Copy link
Contributor

@douglase douglase commented Mar 26, 2018

as discussed in this issue: pydata/numexpr#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
Copy link
Owner

mperrin commented Mar 27, 2018

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
Copy link
Contributor Author

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
Copy link
Owner

mperrin commented Mar 28, 2018

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
Copy link
Owner

mperrin commented Apr 2, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants