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

Turn min/max peak amplitude in GaussianFitter into instance variables #297

Closed SpencerFleming closed 4 years ago

SpencerFleming commented 5 years ago

In GaussainFitter::find_peaks:

if (peak->amp >= 2*max || peak->amp < 10 ||-
    peak->triggering_location > n || peak->triggering_location < 0){
    delete(peak);
    results->erase(iter--);
    } else {
    // set the peak position in the wave
    // this will be wrong if a previous peak was removed for any
    // reason
    peak->position_in_wave = i+1;
 }

This throws away peaks if they have amplitudes greater than 2 times the max, or less than 10. This relies on another variable, so I'll leave this in QA and discuss it with others before making a plan.

SpencerFleming commented 5 years ago
SpencerFleming commented 5 years ago

Seems to be working just fine with the new changes.

SpencerFleming commented 4 years ago

We want to think more about how to convert this into an instance variable. We would like to rename the variable, to make it a more appropriate descriptor. We would like it to be public, or somehow publicly accessable

SpencerFleming commented 4 years ago

This is the current state of the code in question.

 if (peak->amp >= amp_upper_bound*max || peak->amp < amp_lower_bound
         || peak->triggering_location > n
         || peak->triggering_location <0) {
     delete(peak);
     results->erase(iter--);
 } else{
    //set the peak position in the wave
    //this will be wrong if a previous peak was removed for any
    //reason
    peak->position_in_wave = i+1;
 }

Where amp_upper_bound's default is 2, and amp_lower_bound's default is 10.

SpencerFleming commented 4 years ago

2 should be named something like max_amp_multiplier, default to 2, be a float, and be added to the advanced options

cathieO commented 4 years ago

This needs to be added as a command line argument.

SpencerFleming commented 4 years ago

Make sure to add help option

jaredwhite547 commented 4 years ago

Min peak amplitude is in template refactoring. Need to check if we still want a max amplitude option.