Ticket #7261 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Fix PeaksInRegion edge case

Reported by: Owen Arnold Owned by: Owen Arnold
Priority: major Milestone: Release 2.6
Component: GUI Keywords:
Cc: Blocked By:
Blocking: Tester: Peter Peterson

Description (last modified by Nick Draper) (diff)

There might be an edge case that needs fixing for the PeaksInRegion algorithm #7233. When checking that the peaks radius intersects a face, we are actually checking whether it intersects an infinite plane. This could lead to false positives.

If this does turn out to be a problem, and I'll confirm it first using a unit test, then it can be fixed by determining whether a point at the end of the line defined by the distance (already calculated), the peak centre, and the plane normal is within the bounds of that face.

Change History

comment:1 Changed 7 years ago by Owen Arnold

  • Status changed from new to accepted

comment:2 Changed 7 years ago by Owen Arnold

refs #7261. Unit test confirms false positive bug.

Changeset: 758b31ee33552dbd26a1883eb1b407fb71af4e81

comment:3 Changed 7 years ago by Owen Arnold

refs #7261. Fix edge case.

Changeset: adfc19931b69488c80e3c1458699b75e728e3c75

comment:4 Changed 7 years ago by Owen Arnold

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

Tester: Follow the general test instructions given on this ticket #7233.

  • Peak a peak from the file and note it's index as well as it's coordinates in QLab.
  • Open the PeaksInRegion algorithm, set the CoordinateFrame to QLab and construct the extents box such that the peak is not included. Run the algorithm with CheckExtents = false, and confirm that the Intersecting column in the output workspace for that peak index gives False.
  • Now experiment with setting CheckExtents to on, and providing the PeakRadius parameter. It should be logical that the Intersecting column for that peak in the output can be made true if and only if the peak passes through the box defined by the extents.

comment:5 Changed 7 years ago by Owen Arnold

Branch is bugfix/7261_edge_case

comment:6 Changed 7 years ago by Peter Peterson

  • Status changed from verify to verifying
  • Tester set to Peter Peterson

comment:7 Changed 7 years ago by Peter Peterson

  • Status changed from verifying to reopened
  • Resolution fixed deleted

This works as advertised, but there are some compiler warnings associated with line 157 in PeaksInRegion.cpp. Two unused variables "peakCenter" and "distance." Fix this and I'll pass the ticket.

comment:8 Changed 7 years ago by Owen Arnold

  • Status changed from reopened to accepted

comment:9 Changed 7 years ago by Owen Arnold

refs #7261. Remove unused vars.

Changeset: c0d5d0fac0343a24e62ee39a7a1c7179f6806975

comment:10 Changed 7 years ago by Owen Arnold

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

comment:11 Changed 7 years ago by Peter Peterson

  • Status changed from verify to verifying

comment:12 Changed 7 years ago by Peter Peterson

  • Status changed from verifying to closed

comment:13 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:14 Changed 7 years ago by Nick Draper

  • Component changed from Framework to User Interface
  • Description modified (diff)

comment:15 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 8107

Note: See TracTickets for help on using tickets.