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

add Wyant/fringe ordered Zernike basis functions too #236

Open
mperrin opened this issue Aug 27, 2018 · 6 comments
Open

add Wyant/fringe ordered Zernike basis functions too #236

mperrin opened this issue Aug 27, 2018 · 6 comments

Comments

@mperrin
Copy link
Collaborator

mperrin commented Aug 27, 2018

Issue by mperrin
Tuesday Aug 29, 2017 at 19:43 GMT
Originally opened as mperrin/poppy#236


Adds functions for getting Zernike polynomial indices and Zernike basis sets using the so-called fringe or Wyant ordering of the Zernike polynomials. I think I've got this right, but gaaah the literature on this is an inconsistent mess.
Same functionality as the existing Noll-ordering-based code (which this is calling behind the scenes); this just has the polynomials in a different ordering.

For now the polynomials are using the same scaling coefficients as the Noll indices, but I'm not sure if that's the correct thing to do, since Wyant himself has these on his web page without any scaling coefficients (see http://wyant.optics.arizona.edu/zernikes/zernikes.htm)

Could use some review, and still needs unit tests before merging.


mperrin included the following code: https://github.com/mperrin/poppy/pull/236/commits

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by mperrin
Tuesday Aug 29, 2017 at 19:44 GMT


@kvangorkom This PR may be of interest to you. (but definitely feel free to completely ignore this if you're getting busy with other things and don't feel like looking at this now!)

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by mperrin
Tuesday Aug 29, 2017 at 19:48 GMT


Oh, and this PR also adds functions plot_noll_zernikes and plot_wyant_zernikes to make pretty plots of the polynomials. Mostly for debugging purposes but I figured I would add them to poppy.zernike since they seem like useful demos to have present.

unknown

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by kvangorkom
Tuesday Aug 29, 2017 at 20:13 GMT


I put together some code a few months ago to do something very similar! I managed to cobble together an algorithm to map Fringe j -> (n, m) (and the inverse), but there's definitely room for improvement. You're welcome to adapt what I have (or else I can when I get the chance): https://github.com/kvangorkom/zernike/blob/master/zernike.py

One thought from looking at your implementation: Fringe Zernikes, in my experience, tend not to be Noll normalized. There's probably room to argue this, but I wonder if it might be better to disable the normalization implicit in zernike_basis_wyant (but add it as an optional argument). [Edit: woops, I see you raised this point above.]

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by mperrin
Tuesday Aug 29, 2017 at 20:32 GMT


oh nice! I figured there had to be an algorithmic way to do that mapping rather than my hack of a lookup table. I will look at merging in your code for that. Thanks much!

Fringe Zernikes, in my experience, tend not to be Noll normalized. [...] Edit: woops, I see you raised this point above.

That's not a "woops", that's specifically some of the feedback I was hoping for from you. 👍

Also I see your code has piston starting as index 1 as opposed to index 0. I wasn't sure what to do about that - I saw different starting indices in different references. Sigh.

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by kvangorkom
Tuesday Aug 29, 2017 at 20:56 GMT


I've seen both index 1 and index 0, and I'm not sure there's a preferred convention. (I believe both Zemax and CodeV adopt 1, but 4D's 4Sight adopts 0). I suspect being explicit about it is the only way to handle it.

Another complication (which you're likely already aware of) is that Wyant adopts a completely different (n, m) convention (see here). These map onto your (r, l) terms, but my impression is that it's not uncommon to generate Fringe zernikes based on Wyant (n, m, sin/cos) inputs. Robert Gray's MATLAB library handles it this way. I don't know if it's worth attempting to tackle this complication.

@mperrin
Copy link
Collaborator Author

mperrin commented Aug 27, 2018

Comment by coveralls
Tuesday Aug 29, 2017 at 21:35 GMT


Coverage Status

Coverage decreased (-0.9%) to 64.53% when pulling 40ac0269957f2c24f5db4c28e0f777f888efbb7d on fringe_zernike into 569d5c6 on master.

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

1 participant