Ticket #5482 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

MantidMatrix method can return incorrect value for error

Reported by: Russell Taylor Owned by: Anders Markvardsen
Priority: minor Milestone: Release 3.1
Component: MantidPlot Keywords:
Cc: Blocked By:
Blocking: Tester: Russell Taylor

Description (last modified by Russell Taylor) (diff)

The MantidMatrix::dataE method returns 1 if the error at the requested point in the workspace is 0 (around line 570). According to the comment this was a "quick fix" for some fitting problem. The offending commit ([e40b0bd4]) was 4 years ago.

Change History

comment:1 Changed 8 years ago by Russell Taylor

  • Description modified (diff)

comment:2 Changed 8 years ago by Nick Draper

  • Owner set to Anders Markvardsen
  • Status changed from new to assigned

If the quick fix is no longer needed then remove this, otherwise we need to think about the long fix.

comment:3 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.2 to Release 2.3

comment:4 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.3 to Release 2.4

Moved to release 2.4

comment:5 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.4 to Release 2.5

Moved at the code freeze for release 2.4

comment:6 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.5 to Release 2.6

Moved to r2.6 at the end of r2.5

comment:7 Changed 7 years ago by Nick Draper

  • Status changed from assigned to new

comment:8 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.6 to Backlog

Moved to the Backlog after the code freeze for R2.6

comment:9 Changed 7 years ago by Anders Markvardsen

  • Milestone changed from Backlog to Release 3.1

comment:10 Changed 7 years ago by Anders Markvardsen

  • Status changed from new to inprogress

remove hardcoded 0->1 in mantidtable. re #5482

Changeset: 8b4dad369a68245f3ffd1dfcab8a1d7f7870b9ef

comment:11 Changed 7 years ago by Anders Markvardsen

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

As far as I can tell

mantidmatrix.cpp(482): double MantidMatrix::dataE(int row, int col) const

is not use in code.

Also Mantid fitting now has the option "Ignore invalid data" hence if a user wants to fit data with zero errors they can either modify these themselves manually or set the "Ignore invalid data" to true.

comment:12 Changed 7 years ago by Anders Markvardsen

To test: check that systemtests and unit tests passes.

Although not required for passing this ticket, since MantidMatrix::dataE is of course allowed to be used in the Mantid framework, you could check to see if you also find that this method is currently used in the framework

comment:13 Changed 7 years ago by Russell Taylor

  • Status changed from verify to verifying
  • Tester set to Russell Taylor

comment:14 Changed 7 years ago by Russell Taylor

MantidMatrix::dataE is called in MantidMatrix::repaintAll() at lines 903 & 913.

comment:15 Changed 7 years ago by Russell Taylor

  • Status changed from verifying to closed

Merge remote branch 'origin/feature/5482_mantidmatrix'

Full changeset: 33baa82c6cee387634c43513fea3dd27b77b299d

comment:16 Changed 7 years ago by Russell Taylor

The method does appear to be ultimately unused, except that it's exposed to python (though I'm not sure why anyone would want to use it). The code in repaintAll that calls it is, I think, never entered. I created ticket #8503 to look at pruning the dead code.

What is clear is that returning 1 was completely wrong!

Last edited 7 years ago by Russell Taylor (previous) (diff)

comment:17 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6328

Note: See TracTickets for help on using tickets.