Fix HFR calculation, add HFR on focus view, adjust focus UI.
ClosedPublic

Authored by TallFurryMan on Sep 11 2019, 11:18 PM.

Details

Summary

Fixed HFR calculation in the FITSData class, which was accumulating integer instead of double values.
This led to incorrect consolidation of HFR over multiple stars in the focus procedure and incoherent results in full-frame mode.

In order to debug, added HFR values directly in the FITS view, next to circular star marks.
The HFR values are now displayed only for the focus module, they do not appear in the regular FITS viewer.

Took the opportunity to adjust the Focus module UI, so that buttons are bigger and a few UI hiccups are resolved.

Notes:

At the time of this differential, the FITS viewer and the Focus module do not detect the same amount of stars.
Apparently, the "JMIndex" in the case of the FITS viewer is taken from the histogram, and equals 0, causing the detection threshold to be computed from the frame mean ADU, and set too high.
The "JMIndex" in the case of the Focus module is left to its default value 100, causing the detection threshold to be computed from the frame standard deviation, and set properly.

On another subject, the CCD Simulator seems to produce the same SNR even as the optical simulation approaches the optimal focus point.
This causes rendered stars to be smaller, and thus to be missed by the FITS detector. In other words, the more focused the frame is, the less stars are detected.
Because of this problem, the focus procedure sometimes fails with the CCD Simulator.

Test Plan
  • Run an autofocus procedure using the CCD Simulator or a real CCD night test so that a dozen of stars are detected.
  • Verify the HFR consolidated for a particular focuser position is the mean value of HFR registered for detected stars.
  • Verify the procedure is overall more stable than before the change.

Diff Detail

Repository
R321 KStars
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
TallFurryMan created this revision.Sep 11 2019, 11:18 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptSep 11 2019, 11:18 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
TallFurryMan requested review of this revision.Sep 11 2019, 11:18 PM
mutlaqja requested changes to this revision.Sep 12 2019, 6:28 AM

Awesome addition to the module and thank you for catching the accumulator issue.

kstars/ekos/focus/focus.ui
9–10

Width must be less than 800 pixels to conform with the rest of the Ekos interface.

kstars/fitsviewer/fitsview.cpp
955

That seems a bit too large when it is subframed. Also, the number is clipped. Maybe it should be Above the star? or maybe it should be smaller in this case?

957

for(const auto &center: imageData->getStarCenters())

This revision now requires changes to proceed.Sep 12 2019, 6:28 AM
TallFurryMan planned changes to this revision.Sep 12 2019, 10:45 AM
TallFurryMan added inline comments.
kstars/ekos/focus/focus.ui
9–10

Actually, this has no impact on the final UI rendering. That width is only used in the designer, the final width in the application is constrained by the layout rules in the manager. As it happens, the whole view in the designer does not reflect the view in the app, it simply tries its best as it depends on the theme and personalization.

kstars/fitsviewer/fitsview.cpp
955

Originally, I made the font size independent from the zoom level. But noise when zoomed made the string unreadable. So I chose to make the font zoom dependent, but with the generic 1280x1024 resolution, the font was far too small in the 800x600 focus UI :) therefore I multiplied by this arbitrary 3.

But I think what you really mean is that the number is clipped when subframing. Thanks, I'll remove HFR printout when subframing, that's not useful with a single star. Correct understanding?

957

Will fix. I'm not a fan of auto variables though, they have a tendency to leave unverified code behind if items change their type, and new functions change their meaning.

Procedure tested this night, works smoothly again. I'm fixing the UI dimensions, but I'm not sure it currently fits in 800x600 with the icon tab and the bottom log?

Yes the size is actually less than 800 pixels with the simulators running.

I'll share a screenshot in my environment too. In any case, I stand my point about layout constraints, I will make sure the minimal size fits the required dimensions. What is that tool you used on the left?

It's kruler. Anyway, please submit your changes and I'll test the UI again.

I quickly checked on an old version on my lubuntu, Ekos is roughly 835x750 when minimal, because of Capture I think. Thanks for these insights.
By the way, wasn't the relative profile curve on a tab in the past?

Adjust focus UI again.

TallFurryMan planned changes to this revision.Sep 14 2019, 11:52 AM

Rendered HFR only if it can be fully displayed (fixes clipped text).
Used an autovar in the enumeration of star centers.

TallFurryMan marked 6 inline comments as done.Sep 14 2019, 2:20 PM

Focus UI is now smaller than 800x600 when I restrict the manager to that size. I'll tackle the rest of the modules in another diff.

Looking good.

  1. Advanced tab .. maybe we should rename this to "Parameters"?
  2. HFR number.. this can be improved.. in some situation all numbers are tangled up (see screenshot). is color hardcoded or depends on some established color name?

Awesome job, Eric!

I've tested it with simulators and everything looks good. Having the HFR-values displayed makes it much more transparent what the focusing algorithm does.

Maybe one thing maybe needs some polishing. The idea with the tabs for Settings and Advanced are a good idea. I personally would place more parameters into the Settings tab and less under "Advanced". I personally would opt having at least box size and step size under "Settings", maybe tolerance as well.

But this fine tuning. I would publish this diff and issue the fine tuning later.

wreissenberger accepted this revision.Sep 15 2019, 2:04 PM
  1. Advanced tab .. maybe we should rename this to "Parameters"?

That would make "Settings" and "Parameters". Maybe I should replace "Settings" with "Basic" if I move some stuff in from "Advanced"?
I'll try to move stuff around. I would expect for instance SEP parameters to get a new tab.

  1. HFR number.. this can be improved.. in some situation all numbers are tangled up (see screenshot). is color hardcoded or depends on some established color name?

I see. However your frame is a bit edge-case to me, it has the algorithm incorrectly detect many stars garbled in the same place.
I see how I could do, but I'm not sure I want to extend the fits viewer right now with a list of quads containing HFRs that should be sorted, translated and eventually filtered, then rendered...
My night test looks like this, but this is not a cluster, agreed.


Oh and that's 20s Ha exposure, the method works great with that narrowband filter at least. 5 iterations to achieve optimal focus.

Awesome job, Eric!
I've tested it with simulators and everything looks good. Having the HFR-values displayed makes it much more transparent what the focusing algorithm does.

Thanks. Along with the area filter, it's really more stable than the single-star focus procedure in my opinion (with my setup in fact).

Maybe one thing maybe needs some polishing. The idea with the tabs for Settings and Advanced are a good idea. I personally would place more parameters into the Settings tab and less under "Advanced". I personally would opt having at least box size and step size under "Settings", maybe tolerance as well.

I can fit only two more lines with three columns for the first tab. I'll see what I can do.

Updated tabbed UI with clearer groups.

Makes sense?

Good idea, far better!

Should there be some simple label collision-detection to make sure they don't overlap?

The labels are simple drawText calls. There could be a first loop consolidating a list of rectangles bounding labels, then a second pushing labels away from each other but keeping them close to their star marker. But the added value is not high enough for me to implement this now. It'd more important to be able to tweak SEP parameters to detect stars more efficiently I think.

Ok, I guess this could go into a future PR.

mutlaqja accepted this revision.Sep 19 2019, 6:43 AM
This revision is now accepted and ready to land.Sep 19 2019, 6:43 AM
This revision was automatically updated to reflect the committed changes.