spacetelescope / poppy

Physical Optics Propagation in Python
https://poppy-optics.readthedocs.io
BSD 3-Clause "New" or "Revised" License
219 stars 72 forks source link

Numerical noise changes behavior of ThinLens optic #14

Closed mperrin closed 6 years ago

mperrin commented 6 years ago

Issue by josePhoenix Friday Oct 31, 2014 at 17:35 GMT Originally opened as https://github.com/mperrin/poppy/issues/14


While working on adapting the ThinLens optic to use the poppy.zernike library of functions, I noticed that the present implementation of ThinLens has a problem: the radius normalization changes when inserting an intermediate image plane between the pupil and the thin lens.

Test case

import poppy

RADIUS = 3.0 # meters
WAVELENGTH = 4.6e-6 # meters
PIXSCALE = 0.01 # arcsec / pix
FOV = 3.0 # arcsec
NWAVES = 1.0

osys = poppy.OpticalSystem("Testing Zernikes")
circular_aperture = poppy.CircularAperture(radius=RADIUS)
osys.addPupil(circular_aperture)
# uncomment this to add an intermediate plane:
# osys.addImage()
thinlens = poppy.ThinLens(nwaves=NWAVES, reference_wavelength=WAVELENGTH)
osys.addPupil(thinlens)
osys.addDetector(pixelscale=PIXSCALE, fov_arcsec=FOV)

plt.figure(figsize=(8,8))
psf_without_intermediate = osys.calcPSF(wavelength=WAVELENGTH, display_intermediates=True)
plt.tight_layout()

With the addImage() call commented out as above, the resulting PSF looks like:

psf_without

With the addImage() call:

psf_with

Cause

The defocus analytic optic needs to know the extent of the aperture in order to normalize the radius such that rho = 1 at the edge of the illuminated portion. The way this works now, the radius is guessed from the pixels in the aperture.

From poppy/optics.py: ThinLens.getPhasor

y, x = wave.coordinates()
r = np.sqrt(x ** 2 + y ** 2)

max_r = r[np.where(wave.intensity > 0)].max()
r_norm = r / max_r

wave.intensity > 0 is true for all pixels after a single back-and-forth FFT/IFFT, because numerical noise shows up in the zero pixels (~1e-46).

Solution

The proposed solution is to require ThinLens instances to be instantiated with a size keyword argument specifying their radius in meters in the original pupil plane.

It may also be possible to make a better guess at which pixels are "close enough" to zero, and improve the guessing of the aperture radius. This would preserve backward compatibility, but may not be totally foolproof in the case of a grayscale pupil.

It's better to be explicit than implicit, so I'll update POPPY and the corresponding parts of WebbPSF to use a size= keyword argument.

mperrin commented 6 years ago

Comment by mperrin Friday Oct 31, 2014 at 19:37 GMT


Let's make the keyword radius instead of size so it's more explicit what exactly it's measuring. Thanks.

mperrin commented 6 years ago

Comment by josePhoenix Friday Oct 31, 2014 at 20:08 GMT


Yep! Actually had already made that change, but called it "pupil_radius".

Joseph

On Oct 31, 2014, at 3:37 PM, Marshall Perrin notifications@github.com wrote:

Let's make the keyword radius instead of size so it's more explicit what exactly it's measuring. Thanks.

— Reply to this email directly or view it on GitHub.