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
Unexpected result for short-distance Fresnel propagation #194
Comments
@maciekgroch Are you sure the lengths you're using are the same in both tools? What tool produced the second image? I'm not familiar with Fresnel propagation at very short wavelengths. |
I am using a custom-written tool we developed at the university (it's not public unfortunately). I thought it should work for every wavelength. |
Have you looked at the debug log from this propagation? If you have webbpsf installed the easiest way is you can use |
I attempted to run with logging, but didn't see any output from POPPY when running this code. I'm not sure if that's a separate issue, or if there are no logging calls in the code used for this short example... |
Ah, it's
|
That's weird, (edit: this message posted before I got josePhoenix's second post immediately above) |
Here's the test case as a notebook: https://gist.github.com/josePhoenix/edfe9082b0230fb063fda5202d07d6b1 |
Interesting. propagate_direct gives an answer that appears to agree
|
hmm, are we sure that |
I tried the one with
I guess there is something wrong with units conversion🤕 |
@maciekgroch, good catch on the pixel scale, just put in pull request #195 which should fix the units. |
Thank you @douglase for the quick response! Also thank you @maciekgroch for exercising this corner of the code and taking the time to tell us what you found 😄 |
Fix propagate_direct pixel scale units, as noted in #194
@mperrin, for a 800 nm waist at 10nm the Rayleigh length is 200 microns so this is well within the plane to plane regime. but whether the plane to plane code captures the physics is another question. |
yeah thanks for double checking that about the Rayleigh length. I did some more calculations after my post above and came to a similar conclusion, so that's probably not it. But doesn't our |
yeah, I forgot we finished |
Are you sure that you fixed the units issue? What do you think? |
I have been trying to avoid extra .to() calls unless there is a specific reason. they seem like clutter but that is a matter of taste. I did notice today that there are a few .value calls without to(), which could lead to unexpected behavior. astropy simplifies any combination of units to SI units if you call wavefront.pixelscale.decompose() |
Alright then, I have never worked with Astropy before so I didn't know about decompose method. What about division by u.pix? |
I don't want to redefine |
…term in ptp, added unit tests
@maciekgroch, can you check the latest version of master addresses the problems you saw? |
@douglase, Sorry for the late replay, I was away again. I will work on this today. |
Yeah it works perfect! Thank you for help. I think the issue may be closed.
|
@maciekgroch Thanks for your help in identifying and fixing this problem! Much appreciated. :-) |
Wait, upon closer examination I'm not 100% convinced everything is in agreement. It looks like the diffraction patterns at the start and end of this thread still don't quite match: Note the differences in intensity of the various rings. Looking at the code it seems these are both for the same test case wavelength and propagation distance, etc? So that seems to show there is still some disagreement. Do we need to continue tracking this down more? |
I don't have quick access a commercial package at the moment, but I ran PROPER in GDL with the prescription below and got a result that looks consistent with the current POPPY output. @maciekgroch, have you tested your pattern against other models or do you know of any 3rd party published figures we can try to recreate in the two packages?
|
Aha! Great, @maciekgroch thanks for checking that. I had hoped it would be some small difference in calculation parameters like that which would explain the difference. Very glad to hear that my hunch was right! :-) @douglase see above - @maciekgroch edited his comment after figuring out the solution. :-) @douglase now that you've got a PROPER version of this, maybe we could read off a couple values like the peak intensity and add them as additional assertion checks in the new unit test for this? Would make it that much more robust of a test function. |
@maciekgroch that's a relief. Thanks for reporting this and being so responsive with testing. @mperrin what about including a FITS file for the test? |
Just checking, the functionality here has been sorted out and we're only waiting on tests to close the issue? |
I believe it is ready -- I experimented a bit and encountered no problems. |
This issue has been fixed since last October, we just never actually closed the Github issue then. |
User @maciekgroch reports unusual behavior when propagating a 10nm wave a short distance past a circular aperture using the following code:
POPPY produces this intensity pattern after propagating 12 um:
The user's other propagation software produces this pattern:
@douglase, any ideas?
The text was updated successfully, but these errors were encountered: