Ticket #9502 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Issues with ConvertToMD

Reported by: Owen Arnold Owned by: Alex Buts
Priority: critical Milestone: Release 3.2
Component: Diffraction Keywords:
Cc: martyn.gigg@… Blocked By: #8026
Blocking: Tester: Martyn Gigg

Description

Pascal is unable to run the new version of ConvertToMD on his data. The old version ConvertToMDHistoWorkspace (version 1) works.

This could just be down to an over-eager check in run.cpp. In which case, perhaps it should be removed. See email chain below.

We need Pascal to provide the run number and parameters so that the issue can be checked and verified before closing.

From: Gigg, Martyn (Tessella,RAL,ISIS)
Sent: 21 May 2014 9:38 AM
To: Arnold, Owen (Tessella,RAL,ISIS); Buts, Alex (STFC,RAL,ISIS); Manuel, Pascal (STFC,RAL,ISIS)
Subject: RE: ConvertTODiffractionMDWorkspace

You’re correct in that it suggests the first X value is larger than the last X value.
 
I guess in actual fact we don’t really need to check binning of the input data. It just may behave not quite as expected later in some places. I would say that we should just remove the check for start being less than end and leave them stored exactly as they are I the Run object. I don’t think we should sort them internally as I think that should be done on the input workspace beforehand if necessary.
 
Martyn
 
From: Arnold, Owen (Tessella,RAL,ISIS) 
Sent: 21 May 2014 09:26
To: Buts, Alex (STFC,RAL,ISIS); Manuel, Pascal (STFC,RAL,ISIS)
Cc: Gigg, Martyn (Tessella,RAL,ISIS)
Subject: Re: ConvertTODiffractionMDWorkspace
 
I've cc'd Martyn as he seems to be the last author of the code on the run side of things (the bit that is throwing an exception).
 
It seems to be indicating that the x-binning for your input workspace is not running from lowest to highest. 
 
Martyn: Is this a necessary restriction? Presumably the data could internally be reordered, but this would be a large performance hit.
 
Owen.
 
From: Alex Buts <Alex.Buts@stfc.ac.uk>
Date: Wednesday, 21 May 2014 09:05
To: Owen Arnold <owen.arnold@stfc.ac.uk>, "Manuel, Pascal (STFC,RAL,ISIS)" <pascal.manuel@stfc.ac.uk>
Subject: Re: ConvertTODiffractionMDWorkspace
 
Hi, everybody,

I believe this comes from external piece of code, responsible for copying run into into target MD workspace. 

namely:
371 void ConvertToMD::copyMetaData(API::IMDEventWorkspace_sptr mdEventWS, MDEvents::MDWSDescription &targWSDescr) const
{ ....
 
383      const MantidVec & binBoundaries = m_InWS2D->readX(0);
.....
 
401       ExperimentInfo_sptr expt = mdEventWS->getExperimentInfo(i);
402       expt->mutableRun().storeHistogramBinBoundaries(binBoundaries);
 ....
}
 
Either somebody changed something in Run class (I do not know who and why), or input workspace have strange binning. The later explains why it has not been caught by unit tests. Normal debugging information. 


Regards,
Alex




On 21/05/2014 08:36, Arnold, Owen (Tessella,RAL,ISIS) wrote:
Hi Pascal,
 
I've included Alex in this discussion as he developed the new ConvertToMD algorithm, and will know much better what the cause of the problem is here. 
 
Regarding the BinMD issue,  I'll have a look into what's causing that. My impression from your screen hot is that it's down to the way that the slice viewer has chosen the limits.
 
Regards,
Owen.
 
 
From: <Manuel>, STFC <Pascal>, <RAL>, "ISIS)" <pascal.manuel@stfc.ac.uk>
Date: Tuesday, 20 May 2014 17:40
To: Owen Arnold <owen.arnold@stfc.ac.uk>
Subject: RE: ConvertTODiffractionMDWorkspace
 
There are some issues with on the fly rebinning too.
See attached.
 
From: Manuel, Pascal (STFC,RAL,ISIS)
Sent: 20 May 2014 5:26 PM
To: Arnold, Owen (Tessella,RAL,ISIS)
Subject: RE: ConvertTODiffractionMDWorkspace

Found out what it is, we still have to use version 1 (can this be changed asap please, we have been told it would be) and have it in tof (the error message you get is non sensical and there is no reason why it does not do the conversion itself anyway).

From: Manuel, Pascal (STFC,RAL,ISIS)
Sent: 20 May 2014 4:07 PM
To: Arnold, Owen (Tessella,RAL,ISIS)
Subject: ConvertTODiffractionMDWorkspace

Hi Owen,

Do you know if this has been touched again recently ? 
I am getting some garbage about energy bins, see below
ConvertToDiffractionMDWorkspace started
Error in execution of algorithm ConvertToMD:
Run::storeEnergyBinBoundaries - Inconsistent start & end values given, size=4452. Cannot interpret values as bin boundaries.
Error in execution of algorithm ConvertToDiffractionMDWorkspace:
Run::storeEnergyBinBoundaries - Inconsistent start & end values given, size=4452. Cannot interpret values as bin boundaries.

thanks,

P

Change History

comment:1 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Alex Buts

  • Status changed from assigned to inprogress

refs #9502 Spelling errors.

Changeset: c0ad6e51878c11622bfba55d8757092c0caff452

comment:3 Changed 6 years ago by Alex Buts

refs #9502 Methods used to get access to ConvertToMD unit conversion

and verify if this converter actually doing something (converting units)

and some more spelling errors.

Changeset: b7f917a5b486fad4e26dde783ba9937739ebcc1d

comment:4 Changed 6 years ago by Alex Buts

this small script illustrates the error

######################################################################
#Python Script Generated by GeneratePythonScript Algorithm
######################################################################
Load(Filename=r'D:\Data\Mantid_Testing\14_05_22\WISH00028146.nxs',OutputWorkspace='WISH00028146')
#SetUB(Workspace='WISH00028146',a='5.2999999999999998',b='5.2999999999999998',c='5.2999999999999998',u='0,0,1',v='1,0,0',UB='3.816,8.8727,3.8214,0.4641,-6.3844,6.9728,9.3564,-3.5997,-3.2275')
SetUB(Workspace='WISH00028146',a='5.2999999999999998',b='5.2999999999999998',c='5.2999999999999998',u='0,0,1',v='1,0,0',UB='0.04624,0.0150542,0.0872731,0.0675183,-0.0486321,-0.0251251,0.0587435,0.0978839,-0.0288133')
ConvertUnits(InputWorkspace='WISH00028146',OutputWorkspace='WISH00028146d',Target='dSpacing')
ConvertToDiffractionMDWorkspace(InputWorkspace='WISH00028146d',OutputWorkspace='WISH00028146MD',OneEventPerBin='0',OutputDimensions='HKL')


comment:5 Changed 6 years ago by Alex Buts

refs #9502 This should fix it (subj to unit tests & all done properly)

Changeset: 02ef65c7cd9e07a6965507e19327c63b01fe3501

comment:6 Changed 6 years ago by Alex Buts

refs #9502 Fixed isConverted method in Units conversion helper

+ target ws pointer transferred as shared_ptr + fixed unit test

Changeset: fc060e4178cd0ba0abc900b0de9995d2b3383662

comment:7 Changed 6 years ago by Alex Buts

refs #9502 piece of code which selects non-monitor spectra as the

source of the bin boundaries

Changeset: 7016c0816cacaf210a700c99bf3b53b3ff839c1e

comment:8 Changed 6 years ago by Alex Buts

refs #9502 Added lost retrieve bin boundary statement.

Changeset: b404c369af978265e7caabaa5c9bf86098b96ab4

comment:9 Changed 6 years ago by Alex Buts

refs #9502 Merge branch 'master' into bugfix/9502_ConvToMDIssues

(fixing wiki changes)

Conflicts:

Code/Mantid/Framework/MDAlgorithms/src/ConvertToMD.cpp

Changeset: 92539abf3a0dac50ab6bd861eb5a0e4dddfdcdb3

comment:10 Changed 6 years ago by Alex Buts

refs #9502 fixing unit test for bin boundaries

(which now have changed to correct values)

Changeset: 23d7e9860ff8b03c3256b226c6da702b27163e56

comment:11 Changed 6 years ago by Alex Buts

refs #9502 Returning to previous (incorrect) behavior

for event workspace bin boundaries, which should fix live listener test though

Changeset: 32ad087aa94eee743a017f58105a5cce85a30260

comment:12 Changed 6 years ago by Alex Buts

  • Status changed from inprogress to verify
  • Resolution set to fixed
  • Blocked By 8026 added

Should be tested after #8026 only as the branch originates from there.

The reason for error are incorrect or incorrectly arranged bin boundaries. I believe I've fixed the logic and the problem for a histogram workspace but for event workspace it is unclear what to do so, just programming errors are fixed and correct logic remains unclear.

To test one can run test script I supplied above to reproduce problem and check that the conversion runs smoothly after this ticket is applied. One can use smaller/different file from WISH00028146 -- I verified the script reproduces error using MARI11001.raw from test folder.

The error should disappear after the ticket has been applied.

comment:13 Changed 6 years ago by Alex Buts

refs #9502 minor comments

Changeset: 9135d1c0e12772afbc2ec63e313b0a590fe7986b

comment:14 Changed 6 years ago by Martyn Gigg

Restoring pre-commit hook that has been deleted accidentally.

Refs #9502

Changeset: 345232612c070608f54bc564eca5395cf738dfd0

comment:15 Changed 6 years ago by Martyn Gigg

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

comment:16 Changed 6 years ago by Martyn Gigg

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/9502_ConvToMDIssues'

Full changeset: 94d2e658c2e2509fbf365f03f91303901d148561

comment:17 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10345

Note: See TracTickets for help on using tickets.