Repository navigation
Edge Mask shouldn't be saved in OLN files #115
Description
Is your feature request related to a problem? Please describe.
When edge mask is non-zero and also unchecked, sometimes it seems to act as though it is checked. The checkbox and value are found in "configuration" "mirror"
Describe the solution you'd like
Just set the value to zero when unchecked.
Describe alternatives you've considered
Instead, all code that uses edge mask could only use the value if checkbox is checked.
Additional context
https://groups.io/g/Interferometry/topic/102697035
Metadata
Metadata
Assignees
Labels
No labels
Activity
githubdoe commentedon Nov 20, 2023
The edge mask is also saved in the .oln file. I don't think that it should be stored there. It is a value of the mirror configuration. Did I ever think it should go into the .oln file? I don't remember thinking that.
Also I prefer it to never set the value to zero if you uncheck it. I like to be able to uncheck and the recheck it to see how the analysis is changed but don't want to have to reset the value.
githubdoe commentedon Nov 20, 2023
I reviewed my old code and I never saved the edge mask in the .oln file. The more I think about it the more I'm against doing so.
It is a function of the mirror and not of the interferogam.
gr5 commentedon Nov 20, 2023
I never purposely had it save to the oln file. I'll look at the code and see when that change happened.
gr5 commentedon Nov 20, 2023
Looks like you did it back in July of this year.
gr5 commentedon Nov 20, 2023
See your commit comment highlighted in the upper part showing all the commits in blue. Filename is on the right highlighted in blue. Code that saves is circled in red. There is more code to read it back in.
I like that it is in the OLN file because when you come back to this mirror to check the OLN file it seems it should load the config from the OLN file maybe? I'm not 100% sure what the best process is for re-analyzing old igrams. But it sure is convenient to have it in the OLN.
Ideally the OLN would point to an old config "ini" file and strongly suggest the user loads that into the mirror config as well.
I agree that it's basically associated with the config file because if you do it for one igram on a mirror you probably want to do it for all of them.
I suggest for now I simply fix it such that if the checkbox is unchecked it does not save this in the OLN file (just like if it's set to zero it does not save it).
gr5 commentedon Nov 20, 2023
The registry currently stores both (and separately) the aperatureReduction[sic] value and if it is checked.
Somewhere in the code is a bug where if the value is non zero and also it is unchecked, it uses the value anyway (and re-checks the box).
Maybe that is all that needs fixing.
gr5 commentedon Nov 20, 2023
There's lots of other stuff in the OLN file that is also in the mirror config. Right now it warns you when it is changing the mirror config file. Maybe it should be more explicit (e.g. "setting edge mask in mirror config").
githubdoe commentedon Nov 20, 2023
Yes it looks like I did do it. I also created a bug right there. I should have also checked to see if the check box was checked and if the value was also non zero.
However now I think I should never have done that. It is not proper to have it in the outline. It well cause confusion. What happens if one outline has a different value than you are already choosing? It will try to change it. It is a value of the mirror as you agree.
I really don't like it in the .oln file at this time. I don't think it should be there.
githubdoe commentedon Nov 20, 2023
I have looked at the latest .oln file I have written. I don't see anything else from the mirror dialog in it. What things do you see that are?
gr5 commentedon Nov 21, 2023
Sorry. My memory got confused with the wavefront file which does have a few mirror config settings. I was wrong about that.
So right now it's basically 5 things: outer outline, inner outline, regions, DFT filter radius, edge mask.
So I should get rid of the edge mask thing? I can also look over the code to make sure the checkbox is honored everywhere. But perhaps the only "checkbox bug" was the one when saving and loading the outline.
githubdoe commentedon Nov 21, 2023
That sound good. I can't think of why I saved it in the .oln file. Yes it is similar to the outline and it uses the outside outline as it's reference and it is drawn when the outlines are drawn. But as I said it is not related to the igram. We just use the outline processing to display it.
So I think it can be removed from the outline. Then just ignore it if it ever shows up reading old outlines. I hope that is good.
gr5 commentedon Nov 21, 2023
That should be fine. I'll do it this week most likely.
[-]Edge Mask should go to zero when unchecked[/-][+]Edge Mask shouldn't be saved in OLN files[/+]Fixes issue #115 - issues with "edge mask" aka aperture reduction
gr5 commentedon Nov 22, 2023
Okay Dale, I think it's all fixed. PR and code review here: #116
Merge pull request #116 from githubdoe/issue115