Ticket #10224 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

FindPeaks crash in Debug

Reported by: Karl Palmen Owned by: Wenduo Zhou
Priority: major Milestone: Release 3.3
Component: Framework Keywords:
Cc: zhouw@…, anders.markvardsen@… Blocked By:
Blocking: #10052 Tester: Karl Palmen

Description

If you run the following script

ws = CreateSampleWorkspace(Function="User Defined", UserDefinedFunction="name=LinearBackground, \
   A0=0.3;name=Gaussian, PeakCentre=5, Height=10, Sigma=0.7", NumBanks=1, BankPixelWidth=1, XMin=0, XMax=10, BinWidth=0.1)

table = FindPeaks(InputWorkspace='ws', FWHM='20')

in the Debug build, Mantid crashes, but it works OK in Release build. The crash occurs in a loop of FindPeaks::estimatePeakParameters created in the course of ticket #7789, where a call to VECY[100] is made despite it having only 100 elements [0:99].

Change History

comment:1 Changed 6 years ago by Wenduo Zhou

  • Status changed from new to assigned
  • Owner set to Wenduo Zhou

comment:2 Changed 6 years ago by Wenduo Zhou

  • Status changed from assigned to inprogress

comment:3 Changed 6 years ago by Wenduo Zhou

Refs #10224. Fixed the bug caused by array boundary.

On branch feature/10224_findpeaks_bug

modified: ../Mantid/Framework/Algorithms/src/FindPeaks.cpp

Changeset: bdfd8568eb1788cbf281aad116d9cbdea36a798a

comment:4 Changed 6 years ago by Wenduo Zhou

  • Milestone changed from Backlog to Release 3.3

comment:5 Changed 6 years ago by Wenduo Zhou

For tester

Check the change in FindPeaks.cpp. Previously it is not considered if i_max is out of boundary.

Check whether all unit tests and system tests related are passed.

Run the script described in the ticket. You might get error when you run it first time before you merge the change.

Last edited 6 years ago by Wenduo Zhou (previous) (diff)

comment:6 Changed 6 years ago by Wenduo Zhou

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:7 Changed 6 years ago by Karl Palmen

  • Status changed from verify to verifying
  • Tester set to Karl Palmen

comment:8 Changed 6 years ago by Karl Palmen

  • Status changed from verifying to reopened
  • Resolution fixed deleted

It fails in debug mode, because the size of vecY (100) is one less than the size of vecX (101), which you used in the extreme case code.

comment:9 Changed 6 years ago by Karl Palmen

  • Blocking 10052 added

This seems to be causing an occasional failure of the doc-test build. So I'll be temporarily commenting out the print statement in the usage example of FindPeaks.

This ticket must be fixed before #10052 can be fixed.

comment:10 Changed 6 years ago by Wenduo Zhou

  • Status changed from reopened to inprogress

Fixed the bug for extreme case. Refs #10224.

Changeset: eda2e6188fede6514281cd74d9369f64368c2ec8

comment:11 Changed 6 years ago by Wenduo Zhou

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:12 Changed 6 years ago by Karl Palmen

  • Status changed from verify to verifying

comment:13 Changed 6 years ago by Karl Palmen

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Another crash arises within FitOneSinglePeak::genFitWindowsWS() on line 402, which is

    dataY.assign(vecY.begin() + i_minFitX, vecY.begin() + i_maxFitX+1);

Both DataY and VecY have length 100 and I_maxFitX = 100. Perhaps, we need a I_maxFitY = 99.

comment:14 Changed 6 years ago by Wenduo Zhou

  • Status changed from reopened to inprogress

Refs #10224. Considered the extreme case.

On branch bugfix/10224_findpeaks_bug

(use "git reset HEAD <file>..." to unstage)

modified: ../Mantid/Framework/Algorithms/src/FitPeak.cpp

Changeset: 83e69ffe489414ffbe2de9c3b5926c9858dae27e

comment:15 Changed 6 years ago by Wenduo Zhou

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:16 Changed 6 years ago by Karl Palmen

  • Status changed from verify to verifying

comment:17 Changed 6 years ago by Karl Palmen

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/10224_findpeaks_bug'

Full changeset: beea602155a4d6edfbc167362711cadba0572668

comment:18 Changed 6 years ago by Karl Palmen

Merge remote-tracking branch 'origin/bugfix/10224_findpeaks_bug'

Full changeset: beea602155a4d6edfbc167362711cadba0572668

comment:19 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11066

Note: See TracTickets for help on using tickets.