search

BoiseState-AdaptLab / adapt-lidar-tools

Contains code/project notes/ and Data for GEO+CS lidar data processing
GNU General Public License v3.0
8 stars 2 forks source link

guess_peaks Returns NaN fwhm #372

Closed jaredwhite547 closed 4 years ago

jaredwhite547 commented 4 years ago

Specifically, in the method get_fwhm, if ai <= 2*a, (ai is the amplitude of a data point, a is the amplitude of the Gaussian) then we will take the log of a number less than 1, yielding a negative number. We then take the square root of this number, getting NaN.

jaredwhite547 commented 4 years ago

This shows itself in the problem_waveform_9_guess/find test. It wasn't caught because we currently do not validate any of the fwhm/widths in our tests.

jaredwhite547 commented 4 years ago

The new implementation occasionally gives different results. The different result is always better, and tends to be smaller than the previous one.

I am fairly confident the new implementation of the getFWHM function is correct, as I used a CAS to solve for c. The code then converts c to FWHM using the constant.

Tests that changed: (Note that it is only the find tests, because we don't check the widths yet on _guess tests. First peak has a peak# of 1 (i.e. one indexed))

Test                    Peak#   Previous    New     Unit Desired (c)
Split_find              2       9.32        4.86    1.6986436
NayaniClipped1_find     2       9.32        4.86    1.6986436
NayaniClipped2_find     2       8.72        5.19    4.07674464
jaredwhite547 commented 4 years ago

Determined this was due to fwhm being uninitialized sometimes (#366) (I was comparing by using git stash, which included that change). Behavior of both methods is now the same.

jaredwhite547 commented 4 years ago

Fixed in TemplateRefactoring