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

re-importing modules breaks fresnel propagation #257

Open
douglase opened this issue Jun 3, 2018 · 3 comments
Open

re-importing modules breaks fresnel propagation #257

douglase opened this issue Jun 3, 2018 · 3 comments
Milestone

Comments

@douglase
Copy link
Contributor

douglase commented Jun 3, 2018

@mperrin, I found the bit of code that was breaking coronagraph sims during benchmarking.

This is an edge case, but running the following snippet to reload modules after importing causes optics to not be applied while calc_psf is called. The propagation occurs and wavefronts are multiplied by optics, but without rescaling the wavefront pixelscale, so the optic arrays are interpolated to zero size and no flux is observed at the output of the system.

Since the lens power is not applied, this can also change the number of FFTs performed, invalidating benchmark numbers.

This can be diagnosed from the debug statements, where there are no
"DEBUG:------ Optic: " statements. Otherwise the failure is silent.

import importlib
importlib.reload(poppy.fresnel)
importlib.reload(poppy.optics)
importlib.reload(poppy)

Under the new GPU selection paradigm, #250, this hack to set the _USE_CUDA etc.. keywords aren't needed to switch acceleration modes so this shouldn't be problem in practice but is being documented here in case it comes up again.

@mperrin
Copy link
Owner

mperrin commented Jun 3, 2018

Nice debugging to track down where that glitch was coming from. Not so surprising- reloading modules is pretty brittle and unreliable. I’m sure this is related to some calls to isinstance which don’t work as expected after reloading.

@mperrin
Copy link
Owner

mperrin commented Jul 9, 2018

Do we think there is actually any change needed for this, or is it just a matter of 'don't use the reload function, it's not reliable'?

@mperrin mperrin added this to the 0.7.1 milestone Jul 9, 2018
@douglase
Copy link
Contributor Author

douglase commented Jul 9, 2018

The latter is probably OK for now, do we need to include it in a troubleshooting doc or do we feel this issue is this sufficient documentation?

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

No branches or pull requests

2 participants