Ticket #4489 (closed: fixed)

Opened 9 years ago

Last modified 5 years ago

Divide error message is not clear

Reported by: Martyn Gigg Owned by: Keith Brown
Priority: major Milestone: Release 2.6
Component: Framework Keywords: Student,First
Cc: Blocked By:
Blocking: Tester: Samuel Jackson

Description (last modified by Keith Brown) (diff)

The following script produces an error message in Divide.

FileName="POLREF00004711.raw"
Load(Filename=FileName,OutputWorkspace="_Raw")
ConvertUnits(InputWorkspace="_Raw",OutputWorkspace="_Raw",Target="Wavelength")
NormaliseByCurrent(InputWorkspace="_Raw",OutputWorkspace="_D")

Io=CropWorkspace('_D','Io',StartWorkspaceIndex=2,EndWorkspaceIndex=2)
I=CropWorkspace('_D','I',StartWorkspaceIndex=4)

I=mtd['I'];Io=mtd['Io']
R=I/Io

The message states

The sizes of the two workspaces (I_1: 242 spectra, blocksize 1000) and (Io_1: 1 spectra, blocksize 1000) are not compatible for algorithm Divide
Logic Error in execution of algorithm Divide
The sizes of the two workspaces (I_1: 242 spectra, blocksize 1000) and (Io_1: 1 spectra, blocksize 1000) are not compatible for algorithm Divide
Error in execution of algorithm Divide
Execution failed for the algorithm Divide

This looks like it should succeed but the actual problem is that the X values don't match for the input workspaces so a rebin is required. The current check for size compatability only returns a bool. It would better to return an error string and improve the error reporting in cases like this.

Change History

comment:1 Changed 9 years ago by Nick Draper

  • Owner set to Anyone
  • Status changed from new to assigned

comment:2 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.1 to Release 2.2

Moved at end of release 2.1

comment:3 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.2 to Release 2.3

Moved at the end of release 2.2

comment:4 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.3 to Release 2.4

Moved to milestone 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

  • Keywords Student added

comment:8 Changed 7 years ago by Martyn Gigg

  • Keywords Student,First added; Student removed

comment:9 Changed 7 years ago by Martyn Gigg

  • Owner changed from Anyone to Keith Brown

comment:10 Changed 7 years ago by Keith Brown

  • Status changed from assigned to accepted

comment:11 Changed 7 years ago by Keith Brown

Changed checkSizeCompatiblity to std::string to aid error usefulness

BinaryOperation::checkSizeCompatiblity and overridden versions in child classes have been changed to return std::string rather than bool so that a useful error message can be passed back down the call stack before finally being used in throwing std::invalid_argument in BinaryOperation::checkCompatibility. A return of true is now a return of "" (empty string). A return of false is now a string of length 1 or more containing an error about why the algorithm couldn't complete. Functions that used the bool return before have been changed to use checkSizeCompatiblity(lhs,rhs).empty as that is the equivelent of the old boolean return. Removed a load of commented-out code from Multiply.cpp. Indented WeightedMean.cpp and WeightedMean.h as per the guidelines.

Refs #4489

Changeset: 2ffcc5c5c2c255b69cec9d3dcd74a3598d638605

comment:12 Changed 7 years ago by Keith Brown

Streamlined the error throwing and corrected some grammar

BinaryOperation::checkCompatibility no longer uses a stringstream and just throws the message reported by BinaryOperation::checkSizeCompatibility Some error messages were missing some punctuation. One error message was re-worded.

Refs #4489

Changeset: d7f563a67812f89fb4a75f4895e38e4480e418cf

comment:13 Changed 7 years ago by Keith Brown

Tidied up some unneeded comments

Removed a block of commented out code from BinaryOperation.h

Refs #4489

Changeset: a3c2dab6efade1e10154ceda847ce2b074d5f11d

comment:14 Changed 7 years ago by Keith Brown

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

I've checked all the functions with what data i have and they're throwing cleaner and producing more useful errors in the main log.

To Test: In the python script editor, apply all the functions that inherit from BinaryOperation on some workspaces that are known to throw errors on those functions. It is recommended, although optional, to try and get each to return all errors they could possibly return.

comment:15 Changed 7 years ago by Russell Taylor

  • Status changed from verify to reopened
  • Resolution fixed deleted

There's a gcc compiler warning needs fixing (BinaryOperation.cpp line 376): http://download.mantidproject.org/jenkins/job/is_inc_rhel6_develop/1253/warnings14Result/NORMAL/?

Just stick a pair of braces around the subsequent if-else

comment:16 Changed 7 years ago by Keith Brown

Added braces to avoid a warning

Added a pair of braces near BinaryOperation.cpp line 376 to stop an if being ambiguous

Refs #4489

Changeset: 78d26f9e33fd53ab479448baefdde56f32a2547a

comment:17 Changed 7 years ago by Keith Brown

  • Status changed from reopened to accepted

comment:18 Changed 7 years ago by Keith Brown

Added the brackets. Checking build servers now

comment:19 Changed 7 years ago by Keith Brown

  • Status changed from accepted to verify
  • Resolution set to fixed
  • Description modified (diff)

comment:20 Changed 7 years ago by Keith Brown

To Test: In the python script editor, apply all the functions that inherit from BinaryOperation on some workspaces that are known to throw errors on those functions. It is recommended, although optional, to try and get each to return all errors they could possibly return.

comment:21 Changed 7 years ago by Samuel Jackson

  • Status changed from verify to verifying
  • Tester set to Samuel Jackson

comment:22 Changed 7 years ago by Keith Brown

Here is the script i was using

FileName="C:/Users/FXQ83530/Mantid/systemtests/Data/POLREF00004699.raw" Load(Filename=FileName,OutputWorkspace="_Raw") ConvertUnits(InputWorkspace="_Raw",OutputWorkspace="_D",Target="Wavelength") #NormaliseByCurrent(InputWorkspace="_Raw",OutputWorkspace="_D")

Io=CropWorkspace(InputWorkspace='_D',OutputWorkspace='Io',StartWorkspaceIndex=2,EndWorkspaceIndex=2) I=CropWorkspace(InputWorkspace='_D',OutputWorkspace='I',StartWorkspaceIndex=4)

I=mtdI? Io=mtdIo? R=I/Io

comment:23 Changed 7 years ago by Samuel Jackson

  • Status changed from verifying to verify

Merge remote-tracking branch 'origin/feature/4489_divide_error_improve'

comment:24 Changed 7 years ago by Samuel Jackson

  • Status changed from verify to verifying

comment:25 Changed 7 years ago by Samuel Jackson

  • Status changed from verifying to closed

All operations appear to work fine and are producing more verbose strings.

comment:26 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:27 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 5336

Note: See TracTickets for help on using tickets.