search

mljs / global-spectral-deconvolution

Global Spectra Deconvolution + Peak optimizer
http://mljs.github.io/global-spectral-deconvolution/
MIT License
9 stars 8 forks source link

broadenPeaks doesn't prevent overlaps #72

Closed customlogic closed 2 years ago

customlogic commented 2 years ago

Shouldn't broadenPeaks modify the peak's 'x' value in order to prevent overlaps? Currently, it only modifies the width of each peak, but I believe there should be one more step to set the new x value:

  for (let peak of peaks) {
    peak.width = peak.to - peak.from;
    peak.x = peak.from + peak.width / 2; // set x to be in the center of from and to
  }

The tests for broadenPeak seem to assume the x value will remain unchanged, but the peaks they are expecting still overlap.

jobo322 commented 2 years ago

Dear @customlogic, did you check if the joinBroadPeaks function? it may suit your needs, it checks if the peaks are closer than a parameter broadWidth, merge those peaks and optimize the shape parameters of the new peak.

customlogic commented 2 years ago

Thanks @jobo322 , I'll look into joinBroadPeaks

After thinking about it more, I guess my problem is that broadenPeaks creates non-symmetric peaks, but after processing it's impossible to know the offset of "x" within the peak without knowing from and to. from and to were added to the peak in previous versions, but currently only width is returned.

jobo322 commented 2 years ago

@customlogic do you mean the properties left and right? we will back those properties in the structure of Peak1D

lpatiny commented 2 years ago

Should be fixed in Fixed by https://github.com/mljs/global-spectral-deconvolution/commit/42a54fbd907055d233b9b43c85344823f3551bad

Yes it may lead to non-symmetric peaks.

customlogic commented 2 years ago

Thanks, the refactored data structures look really helpful.