GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
224 stars 106 forks source link

Verify wavefront conventions. #1104

Closed jmeyers314 closed 3 years ago

jmeyers314 commented 3 years ago

Aaron Roodman has warned us for years that our OpticalPSF class doesn't match what Zemax predicts. This issue is to take a deep dive and figure out exactly what coordinate system conventions we and Zemax are using, and then either document them, or if merited, adopt Zemax's conventions. Potential points of confusion include:

jmeyers314 commented 3 years ago

To start, Mike recently wrote on another issue:

Oh, one more comment about the conventions in Zernike. I also think some of those might have the 1j in the wrong place, which might explain some of the convention mismatches you were seeing. In particular in _xy_contribution the 1j multiplies the j term, which I think is backwards. I didn't try switching it, but I'd guess it would break some existing unit tests, so you might try to think through whether that should be changed or not.

I looked into this a bit more this afternoon. I think our Zernikes are actually okay. Here's a short function to print out a human-readable polynomial from the _coef_array_xy (which derives from _xy_contribution) and some other details for a given Zernike polynomial:

def Zinfo(j, flipxy=False):
    print(f"j = {j}")
    Z = galsim.zernike.Zernike([0]*j+[1])
    n, m = galsim.zernike.noll_to_zern(j)
    print(f"n, m = {n}, {m}")
    var = (1 if m==0 else 2)*(n+1)
    arr = Z._coef_array_xy/np.sqrt(var)
    print("\n_coef_array_xy")
    print(arr)

    first = True
    out = f"\nsqrt({var}) * ("

    for (i, k), val in np.ndenumerate(arr):        
        if val != 0:
            if not first:
                out += " + "
            first = False
            ival = int(np.round(val))
            if ival != 1:
                out += str(int(np.round(val)))
            if flipxy:
                i, k = k, i
            if i >= 1:
                out += "x"
            if i >= 2:
                out += "^"+str(i)
            if k >= 1:
                out += "y"
            if k >= 2:
                out += "^"+str(k)
    out += ")"
    # Clean up some special cases
    out = out.replace("(-1x", "(-x")
    out = out.replace("(-1y", "(-y")
    out = out.replace("+ -", "- ")
    if out == "sqrt(1) * ()":
        out = "sqrt(1) * (1)"
    print(out)

    print(f"Z_{j}(0, 1) = sqrt({var}) * {int(np.round(Z(0, 1)/np.sqrt(var)))}")
    print(f"Z_{j}(1, 0) = sqrt({var}) * {int(np.round(Z(1, 0)/np.sqrt(var)))}")
    print(f"Z_{j}(1, 1) = sqrt({var}) * {int(np.round(Z(1, 1)/np.sqrt(var)))}")

A few points of comparison: Zemax says Z_2 is 4^(1/2) (p) * COS (A). So assuming that p is sqrt(x^2 + y^2) and A is arctan2(y, x), this is Z_2 = 2 x. The above function, when called with j=2, prints out:

j = 2
n, m = 1, 1

_coef_array_xy
[[0. 0.]
 [1. 0.]]

sqrt(4) * (x)
Z_2(0, 1) = sqrt(4) * 0
Z_2(1, 0) = sqrt(4) * 1
Z_2(1, 1) = sqrt(4) * 1

I'll note that there does seem to be a bit of disagreement in the wild on whether A should be arctan2(y, x) or arctan2(x, y). E.g., slides 6-8 here. The flipxy param above will switch the human readable output from one convention to the other. The paper here prints out a number of Zernikes in the cartesian basis, but adopts this alternate definition for A. Using that and the Zinfo fn, I've verified we're generating the right Zernike functions up to j=36.

Along the way I also checked that horner2d is behaving like I think it should; namely that arr[i, j] is the coefficient of x^i y^j.

So anyway, I think our Zernikes are okay.

aaronroodman commented 3 years ago

I'm pretty sure that the source of the difference is a relative minus sign in the FFT between GalSim and Zemax.

I made my donut code, see https://github.com/aaronroodman/Donut/blob/master/src/DonutEngine.cc#L1221, agree with Zemax, and had to use the FFTW inverse FFT to make them match.

Additionally if GalSim is used in sky coordinates, then obviously there can be an additional transform from using Zemax in focal-plane coordinates - but I guess that isn't intrinsic to GalSim.

jmeyers314 commented 3 years ago

I'm pretty sure that the source of the difference is a relative minus sign in the FFT between GalSim and Zemax.

Thanks @aaronroodman , that could certainly be part of it.

Last night I looked at a bunch of batoid simulations and compared them with Zemax wavefront images. It's pretty clear that a positive Zemax (or batoid) wavefront is leading the reference sphere. I think we can figure out if that convention should go with a forward or inverse FFT empirically by comparing the FFT results to a batoid or Zemax spot diagram (though seeing it in print somewhere would also be nice). I'm much more confident in the coordinates involved in a spot diagram than I am in anything that passes through an FFT. It is important to keep track from which side of the focal plane you view the spots (or the FFT image) though.

The question of sky coordinates, focal plane coordinates, and entrance pupil coordinates is an interesting one. The standard GalSim API defines a surface brightness profile in sky coords. That's not terribly convenient though for an optical PSF, I think. For example, imagine there's a fixed primary mirror defect (or design feature) that produces a PSF artifact that points towards the boresight. Ignoring flexure, that mirror feature will continue to produce a PSF feature pointing towards the boresight no matter where the telescope slews. But that means sometimes it's pointing North, other times East, and so on. So I think you'd need different Zernike coefficients to describe it in OpticalPSF for each case. (Or at least need to rotate the PSF accordingly from a "standard" orientation). This must have come up already for the Roman telescope. @rmjarvis , what's the code pattern there? Or am I thinking about this wrong? Are we doing something for this in Piff optAtmo for the reference Zernikes?

rmjarvis commented 3 years ago

For an equatorial mount, sky coordinates are not too unreasonable for the optical PSF.  But for space or alt-az, there's a rotation to keep track of. 

For the Roman PSF, I added a wcs option to the getPSF function to more easily get that right.  Not sure whether that is also a good option for batoid though.  Seems like the camera rotator would mess up the universality of that kind of approach.

jmeyers314 commented 3 years ago

For an equatorial mount, sky coordinates are not too unreasonable for the optical PSF

Right. "Up = North" ~always for an eq mount. I think I knew that but couldn't put it together yesterday :) ...

jmeyers314 commented 3 years ago

I've been thinking more about this one, mostly for the EQ mount case. See notebook here.

At the moment, it looks to me like there may be a flip in the vertical direction, but not the horizontal direction. That's comparing batoid to galsim though, which is of course different than Zemax to galsim. I do have some zemax <-> batoid unit tests, but I noticed just now that the one for comparing Zernike coefficients only tests along the x=0 field angle axis, so it's possible there's a sign difference when moving off of x=0. If there is a sign difference there, then that probably means there's a flip in the horizontal direction too, which would make the net error a 180 degree rotation which I think is what Aaron suspects.

I don't currently have access to a computer with Zemax (my Zemax computer got booted from the network for lack of a recent security patch and I haven't gone back on site yet to fix it), so maybe Aaron can run some numbers for me? It'd be helpful for debugging to have Zemax Zernike coefficients for, say, several field angles of DECam, preferably ones without confusion-inducing symmetries. Knowing where on the focal plane Zemax says these intersect would also be good for setting up an apples-to-apples comparison.

cwwalter commented 3 years ago

If Aaron can't do it. Then hopefully Bo Xin should be able to help you.

aaronroodman commented 3 years ago

Josh,

Either I can do the Zemax check you’d like, or I can set you up to use the computer I have Zemax loaded on - let me know.

Aaron

On Feb 17, 2021, at 3:00 PM, Josh Meyers notifications@github.com<mailto:notifications@github.com> wrote:

I've been thinking more about this one, mostly for the EQ mount case. See notebook herehttps://gist.github.com/jmeyers314/013b5630c2d444daa9cfd45c5cdec0c4.

At the moment, it looks to me like there may be a flip in the vertical direction, but not the horizontal direction. That's comparing batoid to galsim though, which is of course different than Zemax to galsim. I do have some zemax <-> batoid unit tests, but I noticed just now that the one for comparing Zernike coefficients only tests along the x=0 field angle axis, so it's possible there's a sign difference when moving off of x=0. If there is a sign difference there, then that probably means there's a flip in the horizontal direction too, which would make the net error a 180 degree rotation which I think is what Aaron suspects.

I don't currently have access to a computer with Zemax (my Zemax computer got booted from the network for lack of a recent security patch and I haven't gone back on site yet to fix it), so maybe Aaron can run some numbers for me? It'd be helpful for debugging to have Zemax Zernike coefficients for, say, several field angles of DECam, preferably ones without confusion-inducing symmetries. Knowing where on the focal plane Zemax says these intersect would also be good for setting up an apples-to-apples comparison.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/GalSim-developers/GalSim/issues/1104#issuecomment-780910558, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABARV42DTBMPB2GS5WAZFFLS7RC7TANCNFSM4SRL5LCA.


Prof. Aaron Roodman Dept. of Particle Physics & Astrophysics Kavli Institute for Particle Astrophysics & Cosmology SLAC National Accelerator Laboratory Stanford University

SLAC National Accelerator Laboratory E-mail: roodman@slac.stanford.edumailto:roodman@slac.stanford.edu 2575 Sand Hill Rd. Phone: 650-926-2705 MS 29 Menlo Park, CA 94025 URL: http://www.slac.stanford.edu/~roodman


aaronroodman commented 3 years ago

And… I probably already have what you’d like - I have complete focal plane Zernike maps for DECam...

On Feb 22, 2021, at 9:53 AM, Aaron Roodman roodman@slac.stanford.edu<mailto:roodman@slac.stanford.edu> wrote:

Josh,

Either I can do the Zemax check you’d like, or I can set you up to use the computer I have Zemax loaded on - let me know.

Aaron

On Feb 17, 2021, at 3:00 PM, Josh Meyers notifications@github.com<mailto:notifications@github.com> wrote:

I've been thinking more about this one, mostly for the EQ mount case. See notebook herehttps://gist.github.com/jmeyers314/013b5630c2d444daa9cfd45c5cdec0c4.

At the moment, it looks to me like there may be a flip in the vertical direction, but not the horizontal direction. That's comparing batoid to galsim though, which is of course different than Zemax to galsim. I do have some zemax <-> batoid unit tests, but I noticed just now that the one for comparing Zernike coefficients only tests along the x=0 field angle axis, so it's possible there's a sign difference when moving off of x=0. If there is a sign difference there, then that probably means there's a flip in the horizontal direction too, which would make the net error a 180 degree rotation which I think is what Aaron suspects.

I don't currently have access to a computer with Zemax (my Zemax computer got booted from the network for lack of a recent security patch and I haven't gone back on site yet to fix it), so maybe Aaron can run some numbers for me? It'd be helpful for debugging to have Zemax Zernike coefficients for, say, several field angles of DECam, preferably ones without confusion-inducing symmetries. Knowing where on the focal plane Zemax says these intersect would also be good for setting up an apples-to-apples comparison.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/GalSim-developers/GalSim/issues/1104#issuecomment-780910558, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABARV42DTBMPB2GS5WAZFFLS7RC7TANCNFSM4SRL5LCA.


Prof. Aaron Roodman Dept. of Particle Physics & Astrophysics Kavli Institute for Particle Astrophysics & Cosmology SLAC National Accelerator Laboratory Stanford University

SLAC National Accelerator Laboratory E-mail: roodman@slac.stanford.edumailto:roodman@slac.stanford.edu 2575 Sand Hill Rd. Phone: 650-926-2705 MS 29 Menlo Park, CA 94025 URL: http://www.slac.stanford.edu/~roodman



Prof. Aaron Roodman Dept. of Particle Physics & Astrophysics Kavli Institute for Particle Astrophysics & Cosmology SLAC National Accelerator Laboratory Stanford University

SLAC National Accelerator Laboratory E-mail: roodman@slac.stanford.edumailto:roodman@slac.stanford.edu 2575 Sand Hill Rd. Phone: 650-926-2705 MS 29 Menlo Park, CA 94025 URL: http://www.slac.stanford.edu/~roodman


jmeyers314 commented 3 years ago

Thanks, @aaronroodman. What you have is likely sufficient. The one tricky part is making sure that the meaning of +x and +y is clear. The best way I know to track that is by knowing where in global coords the beam for a particular field angle intersects the focal plane.

jmeyers314 commented 3 years ago

Thinking a bit more about this the past week. Ideally, I think there are two relevant high level goals for OpticalPSF.

  1. Zernike coefficients and pupil obscurations should be defined in "mirror" coordinates.
  2. The resulting GSObject should be defined in equatorial coordinates.

The logic for (1) is that given a particular bending mode on the primary mirror, say, the aberration coefficients shouldn't also depend on where the telescope is pointed.

The logic for (2) is that I think this is how every other GSObject in GalSim behaves (with the possible exception of atmospheric PhasePSFs which suffer similar complications to OpticalPSF). We want obj.xValue(u, v) to give the relative surface brightness u units West and v units North of the object center.

Achieving these goals is straightforward for an EQ mounted telescope, but for an AltAz or space telescope involves some additional rotations.

I think we need to be able to specify the transformation from mirror coordinates to equatorial coordinates in the OpticalPSF constructor. While in general this could be any affine transformation (b/c distortion does matter for computing an FFT PSF), it may be reasonable to limit this to a rotation and possibly a flip. For an EQ scope, for example, suitably chosen x/y axes for the mirror would yield an identity transformation from mirror coords to equatorial coords. For an AltAz scope, the most natural angle is the parallactic angle (but the exact details again depend on how x and y on the mirror are defined). For space, any angle is possible. In summary, I'd suggest something like a new keyword mirror_rot, and potentially mirror_flip and mirror_jacobian to handle these.

Finally, note that I haven't mentioned the camera rotator. To the extent that rotating optics change the wavefront as seen in "mirror" coordinates, that will need to be accounted for in the aberrations input to OpticalPSF. But the rotation of the sensors against the sky can be accounted for entirely by the existing WCS framework. So OpticalPSF doesn't need to know the camera rotator angle explicitly.

rmjarvis commented 3 years ago

We already ran into this a bit with the Roman OpticalPSF, since being a space telescope, it can rotate freely relative to the sky, and the diffraction spikes, even more than the aberrations, need to be fixed with respect to the telescope structure.

The solution I came up with for that was to first construct the OpticalPSF in telescope coordinates. Then project that onto the sky using whatever WCS is currently appropriate for the telescope orientation and rotation. cf. https://github.com/GalSim-developers/GalSim/blob/main/galsim/roman/roman_psfs.py#L262

I think this is actually a pretty general solution, and may even be simpler to implement than adding new mirror parameters.

The only wrinkle is that in the presence of a camera rotator, you would want this WCS to be different (by a rotation) than what you need for the CCDs. So you'd want to use a WCS that is appropriate for the unrotated camera, rather than the one you eventually use for drawing.

jmeyers314 commented 3 years ago

The solution I came up with for that was to first construct the OpticalPSF in telescope coordinates. Then project that onto the sky using whatever WCS is currently appropriate for the telescope orientation and rotation.

Sure, this sounds like a good solution for space. My only comment is that I think it then forces you to use mirror x/y coords that are aligned with image x/y coords. That's essentially the wrinkle you mention above.

I guess the decision is whether or not to enable transformations like this occur inside of OpticalPSF. Enabling increases the odds that any GSObject handed to me has u West and v North, but I guess the existence of wcs.toImage(gsobj) means I shouldn't rely on that anyway...

Okay, new proposal: add text to the OpticalPSF docstring warning that the output will be in telescope coords and users may need to transform to world coords. Then provide examples for how one might do this for equatorial, altaz, and space telescopes, but comment that the possibilities are really wide ranging. This and probably switching this line from fft2 -> ifft2 (after further sanity checking) would then make me happy for this issue.

rmjarvis commented 3 years ago

I think it works for altaz too. Just with the rotator, it's a different WCS you need to pass than the one you use for drawing. But I suspect that's easier to work out correctly (e.g. from LSST DM functions) than to have a new set of parameters with new sign conventions that need to be worked out.

mperrin commented 3 years ago

Hi folks. This is a comment from out of left field but I wanted to let you know we recently did a deep dive through sign conventions for WebbPSF. I can confirm the need for using inverse FFT to match Zemax, and we even have a pretty good understanding of the root causes for that, in terms of choices made by different authors in publications back in the 70s.

Read here for our take on all this, in case it's useful to you: https://poppy-optics.readthedocs.io/en/latest/sign_conventions_for_coordinates_and_phase.html#Sign-Conventions-for-Wavefront-Error-and-Phase

jmeyers314 commented 3 years ago

Thanks @mperrin ! We welcome the comments from left field :). The poppy webpage and analysis look great. I haven't had a change to read through the Wyant and Creath link yet, but that also looks useful.

One potential curveball that I haven't wrapped my head around yet is that the Rubin observatory optics (and I think HSC and DECam also actually) produce an exit pupil behind the focal surface. I.e., a ray that intersects the entrance pupil at positive x will intersect the exit pupil at negative x, flipping the sign. Since this is effectively the same sign as in the Fourier kernel, I wonder if this can make a difference in the calculations. Have you encountered this situation with poppy by chance?

jmeyers314 commented 3 years ago

Noticed this today too: https://sitcomtn-003.lsst.io. It's true what @cwwalter says: 90% of astronomy is just coordinate transforms!

rmjarvis commented 3 years ago

Done #1124