Skip to content

[NOT READY] fix wrong zernike fit in case of (annular obstruction AND aperture reduction) - #372

Draft
atsju wants to merge 2 commits into
masterfrom
JST/368
Draft

[NOT READY] fix wrong zernike fit in case of (annular obstruction AND aperture reduction)#372
atsju wants to merge 2 commits into
masterfrom
JST/368

Conversation

@atsju

@atsju atsju commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

fix #368

@github-actions

Copy link
Copy Markdown

🚀 New build available for commit 828f079
Download installer here

@github-actions

Copy link
Copy Markdown

🚀 New build available for commit 8f8f0ef
Download installer here

@atsju

atsju commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

I fixed the fit (code can be simplified a little) but there are still alot of display issues.

image image image

3D and contourplot display the correct external data (compared to the igram ripples).
Profil plot is actually working well and showing the only ripple it's supposed to show (500 hole and 600 aperture after masking)

What's wrong :

  • 3D plot shows 500 radius instead 300
  • 3D and profile plot : the hole diameter is not 500 (external diameter is at 600)

@atsju

atsju commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

the more I look to the code, the more I see a tight coupling between mirrorDlg and display (profilePlot, contourplot, 3D plot).
It is curently impossible to obtain a correct display without relying on mirrorDlg.

Couple of possible solutions :

  • remove edge mask entirely (it's unreliable right now)
  • copy information used for analysis to wavefront (this proabably means 100% mirrorDlg configs copied and we need a way to check the configs when importing, displaying, ....)
  • analysis only full mirrors and make "edge mask" a per wft display thing only.

It looks like third option is already implemented :
image
So I would advocate for removal of "edge mask" feature as a mirror configuration.

This is a little bit beyond my knowledge right now. I would be happy to get your point of view and ideas.

@gr5

gr5 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Headed to Stellafane in an hour but...

A) I thought edge mask just masked out the edge and set those "pixels" to be null or whatever you want to call it. But still used the same zernike calculations as if the edge was not masked. To me that is probably the easier way to deal with it. You can even process the edge the same way and the same functions where you process regions.

B) But it sounds like it actually shrinks the outer diameter with processing and uses smaller matrixes?

I had assumed the bug was that it does A but incorrectly changes the zernike calculations. But now I'm thinking it might do B? Or it is supposed to do B but it's doing it wrong?

@githubdoe

Copy link
Copy Markdown
Owner

The edge mask is a convivence. It allows one to enter the full diameter of the mirror but then see what the quality would be if one masks the edge. When you mask the edge that also changes the null because the clear aperture of the mirror changes. Sometimes we (me included) get confused with what masking the edge really does and why it changes the null other than just ignoring those masked pixels. I too don't have the time at the moment. But if we were to search back through the groupsio archive I'm sure I talked about it. I remember for sure writing an explanation. I remember there were posts that were specific to it's addition to DFTFringe. Please go do a search for it.

So it is/was important and should not be removed. I think the error is just as the original poster said. That is the null for the anulus should use the masked value as the diameter and not the full mirror diameter.

Yes I'm surprised that you did not understand that the mirror dialog is key to all the calculations. Those values are public because everyone needs to know them and that was the most code efficient writing way to do that. The reason it is key is that those values are used to compute the null and scale the plots.

@githubdoe

Copy link
Copy Markdown
Owner

One way to help find more info about the edge mask would be to look at the version history and find the date of when it was added. Then search groups io around that time. I bet it was discussed then.

@atsju

atsju commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

History:
#115 #116 (partially fixed problems)
#39
first appeared in release 5.1, commit adding it is 1c73705 from april 2021. Previous commit dates from June 2019 so it could have been coded locally anywhere in between.

Most interesting Groups message is probably this one https://groups.io/g/Interferometry/message/30940

When you mask the mirror's edge in the mirror config either because you want to apply a mask on top of the mirror in the telescope or the clear aperture is not the same as your mirror diameter then the program will adjust the artificial null taking the mask into account. Masking the wave front on the other hand just sets that masked area to 0 error and does not adjust the artificial null.

This also confirms waht you said George. Edge mask is using method B. Method A is the "ignore regions" feature.


I agree the feature shouldn't be removed. But looking at the code I would not be confident reloading a saved wft with that feature. It will need fixes. I will think about bast architecture to do it.


Dale wrote

Yes I'm surprised that you did not understand that the mirror dialog is key to all the calculations. Those values are public because everyone needs to know them and that was the most code efficient writing way to do that. The reason it is key is that those values are used to compute the null and scale the plots.

Yes I understand it was a very convenient way of doing it. But I know you also want to be able to load wft from different mirrors. This architecture is bugprone so I absolutelly do not want to add mirrordlg to 3D view and countourview. They do not rely on it today and it's a good thing.
Analysis needs to be separated from display. Display shall not rely on mirrorDlg. There are no pure display settings in mirrorDlg this is already good. What needs to be done is give all necessary information to wft and clean the wft load/store path.


Easiest change could be to consider only the new diameter after analysis.
A 1000 mirror with 200 Edge mask would be a considered a 600 mirror.
This is also what makes most sense from my point of view.
You reload a wft => the loaded mirror is a 600mm.
And it would get rid of the warning when changing edge mask. No display is recomputed on edge mask change. Edge mask influences only at analysis. Display uses only the 600mm information.

I believe this would be a very good compromise.
What do you think ?

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

Successfully merging this pull request may close these issues.

Possible small issue with annular zernike

3 participants