Transformations of WavefrontError and subclasses don't do what is expected

[mperrin]

Issue by corcoted
Wednesday Jul 12, 2017 at 02:57 GMT
Originally opened as mperrin/poppy#229

---

The shift_x, shift_y, and rotation don't behave well for the WavefrontError subclasses. These are inherited from AnalyticOptics to do coordinate transformations on the optics.

For example, trying
ZernikeWFE(radius=1*u.m, coefficients=[0,1,0], shift_x=0.6).display(what='opd')
gives me this:

The aperture seems to be chopped to an American football shape and the center of the opd pattern is at the origin. The expected behavior is to have a circular aperture and its opd shifted over by 0.6 in the x direction.

(The transmission is 1 everywhere.)

[mperrin]

Comment by mperrin
Wednesday Jul 12, 2017 at 14:29 GMT

---

That's definitely a bug. But luckily not a hard one to track down. The shifts weren't getting passed through to the call to the zernike basis function, so the above is an unshifted tilt intersected with a shifted circular mask around it. The reason the shifts weren't getting passed through was related to the use of the zernike.cached_zernike1 caching version for better speed on repeated invocations; that cache isn't set up to handle offset coordinates. I added a check for if there's any offset or rotation, and if so now it will call the regular zernike.zernike1 instead, and with the offset coordinates. Will be slightly slower, but more correct! Seems to be working properly in my tests here. Try now on the latest master and see if it's working better for you.

Meanwhile this reminds me of a related question, whether the transmission should be masked also. A while ago we changed the ThinLens class to make it a subclass of CircularAperture such that the transmission is only nonzero over that aperture. Could do similarly with ZernikeWFE since the Zernike are only valid over the unit circle, but we haven't yet. As I recall the reasoning was that other WFE classes could have different valid apertures (or be valid over any aperture without bounds as in the case of the SineWaveWFE class for instance), so it was more consistent for the API to leave them all defining OPDs but not constraining the transmission. I think that makes sense, but could be open to hearing other arguments. This bug reminded me of the question because ZernikeWFE is using a call to CircularAperture in its internals for masking the OPD to only the valid region, but it doesn't similarly mask the transmission, so it's not entirely consistent.

[mperrin]

Comment by mperrin
Wednesday Jul 12, 2017 at 14:31 GMT

---

[mperrin]

Comment by mperrin
Monday Aug 28, 2017 at 16:23 GMT

---

I believe this issue was resolved back in July with commit 3799489. @corcoted if you have time, can you let me know if this is all working correctly for you now? If so I'll go ahead and close this issue.

[mperrin]

Comment by mperrin
Tuesday Apr 03, 2018 at 15:24 GMT

---

Stale issue, resolved a year ago. Going to close this now.