Ticket #1547 (closed: fixed)

Opened 10 years ago

Last modified 5 years ago

Go beyond Fullprof implementation of Ikeda Carpenter pseudo voigt

Reported by: Anders Markvardsen Owned by: Anders Markvardsen
Priority: major Milestone: Iteration 27
Component: Mantid Keywords:
Cc: Blocked By:
Blocking: Tester: Nick Draper

Description

The current implementation of IkedaCarpenterPV function follows strictly the formulas in Fullprof manual and using the special functions provided by GSL. However, so far this strict implementation has not been satisfactory. Purpose of this ticket is to come up with a way to overcome this problem.

Change History

comment:1 Changed 10 years ago by Anders Markvardsen

(In [7265]) Improved the way height/weight of IkedaCarpenter is calculated and behave with the MantidPlot Fit GUI. re #1547.

comment:2 Changed 10 years ago by Anders Markvardsen

(In [7356]) Drastic speed-up of Ikeda-Carpenter peak shape function. Wiki updated also. re #1547

comment:3 Changed 10 years ago by Anders Markvardsen

  • Status changed from new to accepted
  • Component set to Mantid

comment:4 Changed 10 years ago by Anders Markvardsen

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

comment:5 Changed 10 years ago by Nick Draper

  • Milestone changed from Iteration 26 to Iteration 27

Bulk move of tickets to iteration 27, if your ticket is essential for Iteration 26 then move it back.

comment:6 Changed 10 years ago by Nick Draper

  • Milestone changed from Iteration 27 to Iteration 26

Sorry I didn't mean to move these ones reverting back to It 26

comment:7 Changed 10 years ago by Martyn Gigg

  • Status changed from verify to verifying
  • Tester set to Martyn Gigg

comment:8 Changed 10 years ago by Martyn Gigg

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Tested with HRPD data and the fit seems reasonable and fast.

I've uncovered one crash, on Linux at least, relating the Ikeda Carpenter.

To repeat:

  • Load HRPD39191.raw and plot spectra 50;
  • Zoom in on a peak;
  • Click the fit button and add an Ikeda Carpenter function;
  • Click a sensible centre point;
  • Now click on "Find Peaks" and MantidPlot dies.

The first 3 frames in the traceback on the debugger give:

Mantid::API::MatrixWorkspace::getAxis (this=0x0, axisIndex=@0x7fffffffc2e8)
    at API/src/MatrixWorkspace.cpp:541
541	      if ( axisIndex < 0 || axisIndex >= static_cast<int>(m_axes.size()) )
(gdb) bt
#0  Mantid::API::MatrixWorkspace::getAxis (this=0x0, axisIndex=@0x7fffffffc2e8)
    at API/src/MatrixWorkspace.cpp:541
#1  0x00007fffdad8f025 in Mantid::CurveFitting::IkedaCarpenterPV::calWavelengthAtEachDataPoint (
    this=0x3ffedb0, xValues=0x7fffffffc598, nData=@0x7fffffffc5ac)
    at CurveFitting/src/IkedaCarpenterPV.cpp:111
#2  0x00007fffdad91429 in Mantid::CurveFitting::IkedaCarpenterPV::constFunction (this=0x3ffedb0, 
    out=0x7fffffffc5a0, xValues=0x7fffffffc598, nData=@0x7fffffffc5ac)
    at CurveFitting/src/IkedaCarpenterPV.cpp:199
#3  0x00007fffdad91d46 in Mantid::CurveFitting::IkedaCarpenterPV::height (this=0x3ffedb0)
    at CurveFitting/src/IkedaCarpenterPV.cpp:58

which suggests to me that in the IkedaCarpeterPV the 'm_workspace' pointer is NULL for some reason.

I'm reopening the ticket mainly because it crashes. Anders, if you feel this is more Roman's thing then feel free to pass it to him. Considering it's a crash though I think it's worth looking in to.

comment:9 Changed 10 years ago by Nick Draper

  • Milestone changed from Iteration 26 to Iteration 27

comment:10 Changed 10 years ago by Roman Tolchenov

(In [8193]) Added missing call to IFunction::setWorkspace in FitPropertyBrowser. re #1547

comment:11 Changed 10 years ago by Anders Markvardsen

(In [8795]) Phuuu been swearing at the computer and GSL for a number of days. Good news GSL OK, culprit was memory leak when calculate numerical derivatives. re #1547.

comment:12 Changed 10 years ago by Anders Markvardsen

(In [8811]) 1) Now only convertValue() unit method in IFunction. 2) Spotted a missing result-unit="TOF2 for bank one in GEM IDF 3) Removed warning message that IkedaCarpenter can only be used with TOF 4) Added IkedaCarpenterPVTest back in

Added code that: one way to speed up IkedaCarpenter further would be:

note if a version of convertValue was added which allows a double* as first argument then could avoid some copying above plus only have to resize m_wavelength when its size smaller than nData

But this has to be for another day.

When testing this function consider using focussed GEM data (e.g. test/nexus/focussedGEM38370_TOF.nxs) since at present it is only for the GEM instrument that IkedaCarpenter parameter are currently pre-set.

re #1547.

comment:13 Changed 10 years ago by Anders Markvardsen

  • Status changed from reopened to accepted

comment:14 Changed 10 years ago by Anders Markvardsen

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

comment:15 Changed 10 years ago by Anders Markvardsen

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:16 Changed 10 years ago by Anders Markvardsen

(In [8924]) Just discovered that i forgot to delete a cout line. re #1547.

comment:17 Changed 10 years ago by Anders Markvardsen

  • Status changed from reopened to accepted

comment:18 Changed 10 years ago by Anders Markvardsen

(In [8971]) Removed commented out code in header file no longer needed. re #1547

comment:19 Changed 10 years ago by Anders Markvardsen

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

comment:20 Changed 10 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester changed from Martyn Gigg to Nick Draper

comment:21 Changed 10 years ago by Nick Draper

  • Status changed from verifying to closed

testing of ikeda carpenter passed. Verified using a fit of 1 background and 11 IK peaks.

v1.1.9494

Problem with findpeaks button raised as #2418

comment:22 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 2394

Note: See TracTickets for help on using tickets.