<table><tr><td style="">murveit updated this revision to Diff 79350.<br />murveit added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28477">View Revision</a></tr></table><br /><div><div><p>Added a significant bug fix and speedup.</p>

<p>Bug fix. There is a significant bug in the findSEPStars code that is fixed here.<br />
Without this fix, uint16 samples, which is likely the most frequent data type used,<br />
are treated as int16 and thus, sample values greater than 32K are treated as negative numbers.<br />
This results in the calculation of incorrect much larger HFR values for these stars.<br />
This general short/ushort issue was clearly seen before in KStars and was fixed in the privateLoad method, see<br />
line 264 in fitsdata.cpp::PrivateLoad(), where the comment says "read SHORT image as USHORT".<br />
Without this fix, KStars would not work. The fix to findSEPStars() is to reference the m_DataType<br />
variable, already correctly setup in privateLoad(), instead of directly referencing the fits header field.<br />
In the future, if another approach is to be used for differentiating shorts and ushorts, <br />
it should be addressed in privateLoad() and not in findSEPStars().</p>

<p>Speed Up. In profiling findSEPStars(), I found that the lion's share of the time (when run<br />
on sub exposures -- e.g. not very short ones that might be used for focus, but rather exposure<br />
times that would be used for subs) was taken by the calls to sep_flux_radius(). The radius is <br />
calculated for each star, even though, below in the method, only the top 100 widest stars were selected.<br />
I chose to use the size of the star's oval radii instead of width, and this is output by the star detection<br />
so sep_flux_radius only needed to be called on the stars chosen (e.g. 100) instead of all (e.g. 500-1000).<br />
This oval size is very closely related to the HFR, so we get about the same set of stars is had we <br />
selected by largest HFR.</p>

<p>I will follow up with further improvement to the SEP HFR calculations in future PRs.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R321 KStars</div></div></div><br /><div><strong>CHANGES SINCE LAST UPDATE</strong><div><a href="https://phabricator.kde.org/D28477?vs=79254&id=79350">https://phabricator.kde.org/D28477?vs=79254&id=79350</a></div></div><br /><div><strong>BRANCH</strong><div><div>hfr-display-fix (branched from master)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28477">https://phabricator.kde.org/D28477</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>kstars/fitsviewer/fitsdata.cpp<br />
kstars/fitsviewer/fitstab.cpp<br />
kstars/fitsviewer/fitsview.cpp<br />
kstars/fitsviewer/fitsviewer.cpp</div></div></div><br /><div><strong>To: </strong>murveit, mutlaqja, TallFurryMan<br /><strong>Cc: </strong>kde-edu, narvaez, apol<br /></div>