Ticket #9426 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

The height-method of Lorentzian does not return the peak height

Reported by: Michael Wedel Owned by: Roman Tolchenov
Priority: critical Milestone: Release 3.2
Component: Framework Keywords: CurveFitting
Cc: Blocked By:
Blocking: Tester: Michael Wedel

Description

The current implementation of CurveFitting::Lorentzian::height does not return the peak height, but instead the "Amplitude" parameter, which is the area. This makes the following test fail: 

Lorentzian lor;
lor.initialize();
lor.setHeight(2.0);
lor.setCentre(3.0);

std::vector<double> x(1, lor.centre());
std::vector<double> y(1, 0.0);

lor.function1D(y.data(), x.data(), 1);

TS_ASSERT_EQUALS(y[0], lor.height());

Since it was mentioned in #7545 that the function should be based on area, a conversion for accessing this property via height/setHeight should be performed.

Attachments

fixed_screenshot.png​ (59.7 KB) - added by Michael Wedel 6 years ago.
Lorentzian height estimation working after fix

Change History

comment:1 Changed 6 years ago by Michael Wedel

  • Milestone changed from Backlog to Release 3.2

comment:2 Changed 6 years ago by Nick Draper

  • Priority changed from major to critical
  • Owner set to Roman Tolchenov
  • Status changed from new to assigned

comment:3 Changed 6 years ago by Roman Tolchenov

  • Status changed from assigned to inprogress

Re #9426. Changed height() and setHeight() methods of Lorentzian.

Changeset: 59fb2e734e3653189fbfa34a7159b71f8eab4d6d

comment:4 Changed 6 years ago by Roman Tolchenov

To test:

  1. Plot a spectrum.
  2. Switch on the Peak Picker Tool.
  3. Add a Lorentzian.
  4. Plot guess from Fit Browser / context menu.
  5. Vary the peak parameters using both the picker tool and the browser.

The vertical line showing the peak's height should always touch the curve at the maximum.

comment:5 Changed 6 years ago by Roman Tolchenov

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

comment:6 Changed 6 years ago by Michael Wedel

  • Status changed from verify to verifying
  • Tester set to Michael Wedel

comment:7 Changed 6 years ago by Michael Wedel

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/9426_lorentzian_height'

Full changeset: ​915827555ff353f056dd445c5c0b54c8bce5c72a

Changed 6 years ago by Michael Wedel

Lorentzian height estimation working after fix

comment:8 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​10269

Note: See TracTickets for help on using tickets.