ConvertToMD should determine limits if not supplied

[Nick Draper]

ConvertToMD should use the Helper algorithm internally if the defaults are left blank, or not supplied.

[Alex Buts]

refs #7529 Initial changes to convertToMD

Changeset: 940da6b2436bc75920cd4b317de2372c29f61706

[Alex Buts]

refs #7529 Mainly finished -- unit tests left

Changeset: 17f3840a7d52c99632768f9627e62d1e9dfd5ca6

[Alex Buts]

refs #7529 unit tests modified for 4D data

Changeset: 1039026eb8dd289e95900be1488724b5cdb14137

[Alex Buts]

refs #7529 fixing "Unused Q-mode warning"

And added a template for 5D unit test

Changeset: db3706d16f3416ff1d8dfc52c90e692871d2815e

[Alex Buts]

refs #7529 fixing ConvertToMDComponentsTest

Changeset: 4224bad299906ab20741df2049b7c0bdf1d4e720

[Alex Buts]

refs #7529 Doxygen warning

Changeset: 9d6ea35268a88ea3aa0549abf9ad01cf6224fdf0

[Alex Buts]

refs #7529 Added test for 5D and fixed some dependencies.

and invalid properties

Changeset: 827f735d1e1ef646894c99d9249ffc96a0103b09

[Alex Buts]

refs #7529 Changes to WIKI reflecting changes to code

Changeset: 7a4ba269cf3775fb5e160055794d867b580ce15a

[Alex Buts]

To tester:

Ticket purpose is achieved: ConvertToMD uses ConvertToMDHelper to get default min-max values if not supplied.

Tester should use GUI and script to try to fool the ConvertToMD to produce some rubbish. ConvertToMDHelper uses the conversion algorithm which is different from ConvertToMD so I will be surprised if it is not possible.

Pay attention to indirect and elastic instruments and time series as additional dimensions as unit tests run for Direct Inlastic and single number log value as additional dimension.

Possible point of failure can be some exotic coordinate system for Q3D transformation e.g. Q_lab or Q_sample (HKL seems tested)

[Andrei Savici]

Load(Filename='CNCS_7860_event.nxs',OutputWorkspace='CNCS_7860_event')
ConvertUnits(InputWorkspace='CNCS_7860_event',OutputWorkspace='CNCS_7860_event',Target='DeltaE',EMode='Direct',EFixed='3')
Rebin(InputWorkspace='CNCS_7860_event',OutputWorkspace='CNCS_7860_event',Params='-2.9,0.05,2.9',PreserveEvents='0')
SetGoniometer(Workspace='CNCS_7860_event',Axis0='25,0,1,0,1')
SetUB(Workspace='CNCS_7860_event',a='3',b='3',c='3',gamma='120')

print ConvertToMDHelper('CNCS_7860_event','Q3D','Direct','Q')
(array([-2.89064127, -2.89064127, -2.89064127, -2.9       ]), array([ 2.89064127,  2.89064127,  2.89064127,  2.9       ]))
print ConvertToMDHelper('CNCS_7860_event','Q3D','Direct','HKL')
(array([-1.38017954, -1.38017954, -1.38017954, -2.9       ]), array([ 1.38017954,  1.38017954,  1.38017954,  2.9       ]))

No matter what I choose for ConvertToMD (say Q_sample) I get the limits corresponding to HKL. Not correct!!!

Also, I have no idea what are the correct limits for Orthogonal HKL or Q in lattice units, but I cannot test that since I always get the limits corresponding to HKL.

[Alex Buts]

refs #7529 Hopefully sorts out issue identified by tester

Changeset: fd70ed899915323388c56d7070e489a7ef8ba9c9

[Alex Buts]

refs #7529 unit test for parameters transformation from ConverToMD

To ConvertToMDHelper

Changeset: e64cd4665dd1206dda3a2654d271998f8c44dc69

[Alex Buts]

refs #7529 Probably this is correct relation

between ConverToMD and ConvertToMDHelper

Changeset: 5fc4397c2fe1c8148bc706cb57141048c9ba9203

[Alex Buts]

refs #7529 fixing unit tests

Changeset: a2ed59c548449b6f0a64360639d0aee45ba94a1e

[Alex Buts]

refs #7529 trying to fix unix error

Changeset: 0b72861b9530607644ef04c3c7c4ce043563d546

[Alex Buts]

Fixed issues identified by tester, but further tests are necessary as relation between Q in ConvertToMDHelper and other Q=parameters in ConvertToMD is not straightforward.

I hope I have guessed it correctly and made explicit in the function, which is unit tested by commits fixing the error, but all this should be thoroughly tested.

[Keith Brown]

No matter what settings i use in ConvertToMD i get limits corresponding to Q.

So it seems the problem from the last test has been inverted

[Alex Buts]

refs #7529 Hopefully it fixes it

Changeset: 4b84bef769a02cc114d5c6477a6f534f6775bdc1

[Alex Buts]

Problem with this ticket is two independent algorithms with two independent sets of parameters.

First of all, tester should pay attention to correspondence table specified in:

ConvertToMDComponentsTest.h, row 287 and below.

and check if he agrees with it. Then limits produced by ConvertToMD should be verified against limits produced by ConvertToMDHelper and then look at results obtained for all test situations.

[Peter Peterson]

Re #8023 and #7529. commenting out conflicting test branches.

Changeset: 619f24ff52fb781a7702801309dcadafbe8edfb2

[Andrei Savici]

I am not sure the limits are correct for the OrthogonalHKL and Q in lattice units. The "table" in the test is very confusing. Maybe it works, but here is my suggestion: document what each of these options mean and what is the formula to get to the desired unit

[Alex Buts]

well, this job should be done by ConvertToMDHelper algorithm. This algorithm should understand the same options as convert to MD, but apparently it does not, so I have to improvize and do some adapters. ConvertToMDHelper needs substantial changes and that is where I have to eventually go.

[Alex Buts]

refs #7529 Started new ConvertToMDHelper

which would use ConvertToMD core as the main part of the algorithm

Changeset: 8aa4d64c2264138d7f0b443e7e0f3041a260d250

[Alex Buts]

refs #7529 Small further changes

(not in any way completed)

Changeset: ef7371a144b3af63de382387dd01d5041b621ae7

[Alex Buts]

refs #7529 General framework for ConvertToMDHelper2.cpp

probably not yet working correctly

Changeset: 17b287e65ce795a879a13d2bf1daebbdc4c54763

[Alex Buts]

refs #7529 Get rid of test for ConvertToMDHelperParam

as the function has gone

Changeset: 34f464a2f206a2bb779e8f28a6b528bfcecf4857

[Alex Buts]

refs #7529 Enabled "distribution" mode conversion for matrix workspace

Changeset: 239b8d24255464fc77cad9bd2886ccda2627df60

[Alex Buts]

refs #7529 Something working

but may be incorrect and properties are not copied correctly

Changeset: 5bcf30f283adec82afed6f897effccc2423e9a0a

[Alex Buts]

refs #7529 All seems work except some tests

which can be incorrect with new changes.

Changeset: 36747034478d70775ab1d329dcc20544588ab9be

[Alex Buts]

refs #7529 All seems fixed

Changeset: e7cad7115ee911904dda432b84db1ba5585e2917

[Alex Buts]

refs #7529 Merge conflicts with develop

Merge branch 'feature7529/ConvertToMD_Autolimits' into develop

Conflicts:

Code/Mantid/Framework/MDAlgorithms/src/ConvertToMD.cpp Code/Mantid/Framework/MDAlgorithms/test/ConvertToQ3DdETest.h

Changeset: d3058838987722d653ecbb6559802089b9bc263e

[Alex Buts]

refs #7529 fixing wrong merge

Changeset: 035f56e84a889bffc4a3de887e31c6258c974671

[Alex Buts]

refs #7529 MAC is different ....

Changeset: 6f51bcb00ffb1e7c73e1cfb938dade55a167ffc6

[Alex Buts]

refs #7529 fixing typo

Changeset: 1ce46356e1cb3616654809cea67cba8f81c119eb

[Alex Buts]

This is not incremental change

As much as I like old convertToMDHelper for its simplicity and physical meaning it uses different set of properties vrt ConvertToMD and identifying correspondence between two sets of these properties is non-trivial task. In addition, maintaining the resulting code would be nightmare.

This is why I wrote another version of convertToMDHelper, which uses convertToMD core to calculate min-max values on the simplified min-max workspace.

These min-max values are not global for a method as before but are the values, possible for this particular instrument configuration.

I tried to build a spherical model instrument which would allow ConvertToMDHelper2 to behave like ConvertToMDHelper1, but realized that to make it reliable, one needs to orient its detectors along UB matrix eigenvectors (or eigenvectors of its inverse) . As I am not sure that this is at all necessary, I would accept another ticket to do that if somebody is interested.

Tester can verify that real events obtained from ConvertToMD are always within the limits, obtained from ConvertToMDHelper v2. As the same factory method used to calculate limits and events itself, I believe it should always be correct even if transformation or scale changes, which may be necessary in the nearest future.

[Andrei Savici]

Please make sure there is ample documentation about calculating the expected min/max values, especially for Orthogonal HKL and Q in lattice units, and also for calculating |Q| max and HKL. The documentation must be available in the algorithms wiki sections

[Alex Buts]

refs #7529 Wiki and comments

Changeset: d52f11c78289b797ad1a176ec49d4b8205be5c07

[Alex Buts]

The advantage of this approach to identifying min-max is that you do not care what transformation is used by convertToMD. You just use the same transformation as convertToMD.

The only property of this transformation is that it is smooth transformation with 0 is possible single special point (taken care for, though not sure it is relevant). And all transformations in neutronic are smooth. I've borrowed this approach from Horace.

If you want to identify theoretical possible limits for the transformation as convertToMDHelper1 does, you, of course, need to analyse the transformation itself, which is different and much more complex task.

[Alex Buts]

refs #7529 Minor tidiness

Being a bit more careful instead of thinking.

Changeset: 6150eeeebf93ea74fd975b9e1e69dcd281422dd8

[Russell Taylor]

There are compiler warnings needing fixing: ​https://builds.sns.gov/job/ornl_clean_rhel6_develop/2431/warnings16Result/?

[Alex Buts]

refs #7529 fixing compiler warnings on Nix

Changeset: d026d146601a3234211d49fa88f99586bc2972ed

[Owen Arnold]

I'm not sure about this. I tried the following simple script and it's producing different results.

ws = Load(Filename='CNCS_7860_event')
AddSampleLog(Workspace=ws,LogName='Ei',LogText='3',LogType='Number')
SetUB(Workspace=ws,a='1.4165',b='1.4165',c='1.4165',u='1,0,0',v='0,1,0')        
AddSampleLog(Workspace=ws,LogName='Psi',LogText=str(0),LogType='Number Series')
SetGoniometer(Workspace=ws,Axis0='Psi,0,1,0,1')
print ConvertToMDHelper(InputWorkspace=ws, QDimensions='Q3D', Version=1)
print ConvertToMDHelper(InputWorkspace=ws, QDimensions='Q3D', Version=2)

gives different results between your algorithm and Andrei's:

(array([-0.64239052, -0.64239052, -0.64239052, -2.61552638]), array([ 0.64239052,  0.64239052,  0.64239052,  2.61552638])) #Andrei's algorithm
(array([ 0.7765075 , -0.42877218, -0.12154514,  2.58494568]), array([ 1.49309397,  0.34535059,  0.12154514,  2.62552404]))#Alex's algorithm

[Nick Draper]

The differences need to be understood and explained before we could continue testing this ticket.

[Alex Buts]

As I've briefly commented above, new and old ConvertToMD helper legitimately produce different results. Old ConveretToMD helper was provided min-max results global for a method e.g. if one converts to Q3D, the limits calculated by this algorithm were related to an instrument, which covers 360* in all directions. Unfortunately, it does not covers (uses) all transformations which converts to MD does and if convertsToMD changes, correspondent changes to ConvertToMD helper have to be done manually by a person who understands these changes.

New convertToMD helper works much simpler. It just calculates min-max limits on given instrument using the same factory and transformation which ConvetToMD uses. This is why for e.g. maps it will produce q-results covering angles -30+180 in X directions and whatever (+-30 seems) in Y direction. But it uses the same transformation as convertToMD, so the changes to this transformation will be automatically deployed by ConvertToMDHelper.

[Alex Buts]

refs #7529 A bit more wiki description.

Changeset: 66b97b0cd4946d2b3ce983be68c6e13d5fe1de02

[Owen Arnold]

Inelastic case needs some further investigation (runs script below). Plotted images and limits look very different. No negative dE for example. Seems to work OK for elastic case.

WS_Name='CNCS_7860_event'
Load(Filename=WS_Name,OutputWorkspace=WS_Name)
AddSampleLog(Workspace=WS_Name,LogName='Ei',LogText='3',LogType='Number')
SetUB(Workspace=WS_Name,a='1.4165',b='1.4165',c='1.4165',u='1,0,0',v='0,1,0')   
AddSampleLog(Workspace=WS_Name,LogName='Psi',LogText=str(0),LogType='Number Series')
SetGoniometer(Workspace=WS_Name,Axis0='Psi,0,1,0,1')
print ConvertToMDHelper(InputWorkspace=WS_Name, Version=1)
print ConvertToMDHelper(InputWorkspace=WS_Name, QDimensions='Q3D',QConversionScales='HKL',dEAnalysisMode='Direct',Version=2)

minValues = [-3, -3, -3, -1]
maxValues = [3, 3, 3, 3]

auto_limits = ConvertToMD(InputWorkspace=WS_Name,QDimensions='Q3D',QConversionScales='HKL',OverwriteExisting=0,dEAnalysisMode='Direct',SplitInto="20,20,1,1")
manual_limits = ConvertToMD(InputWorkspace=WS_Name,QDimensions='Q3D',QConversionScales='HKL',OverwriteExisting=0,dEAnalysisMode='Direct',SplitInto="20,20,1,1",MinValues=minValues,MaxValues=maxValues)

plotSlice(auto_limits)
plotSlice(manual_limits)

[Nick Draper]

I don't think this will be able to make this release, moving to R3.1

[Alex Buts]

refs #7529 Generally fixes the issue with different Units

but more testing is necessary

Changeset: 91cee5ae119178cde5f0b28041427769ca75ab94

[Andrei Savici]

I was looking at the way you calculate limits for Q, for inelastic case, using deltaE=(min,0,max). That is a bad idea for calculating |Q|. The minimum might not be one of your choices. Look at a particular example:

• instrument has only 1 detector at 45 degrees, and say

  from numpy import *
  ei=100
  de=arange(-50,99)
  ef=ei-de
  ki=sqrt(ei/2.072)
  kf=sqrt(ef/2.072)
  q=sqrt(ki*ki+kf*kf-2*ki*kf*cos(radians(45.)))
  print q[de.argmin()]
  print q[where(de==0)]
  print q[de.argmax()]
  print min(q)
  print de[q.argmin()]
  

Q(de=-50) is 6.0879, Q(de=0) is 5.3171, Q(de=98) is 6.29088. Obviously you would choose Qmin=5.3171. But as you can see, the minimum of q is 4.9123, and it occurs at de=50

[Alex Buts]

Thanks, Andrei,

I've realized this and doing my recent changes to address the issue. As Nick decided to move this ticket to 3.1 and I have other job to do (which I've substantially abandoned), it will be some time until I deal with this.

Your example would be great test to see if I did my job correctly.

The idea I trying to implement is monotonous ranges of transformations. It is not difficult in each particular case but as there are plenty such cases, a lot of unit tests have to be written.

[Alex Buts]

refs #7529 further enhancements (not finished)

mainly introducing monotonic ranges method in Units class

Changeset: 7a07304c431dcfbb4bbec97b77d21afda098ddd4

[Alex Buts]

refs #7529 Initial changes to convertToMD

Changeset: 940da6b2436bc75920cd4b317de2372c29f61706

[Alex Buts]

refs #7529 Mainly finished -- unit tests left

Changeset: 17f3840a7d52c99632768f9627e62d1e9dfd5ca6

[Alex Buts]

refs #7529 unit tests modified for 4D data

Changeset: 1039026eb8dd289e95900be1488724b5cdb14137

[Alex Buts]

refs #7529 fixing "Unused Q-mode warning"

And added a template for 5D unit test

Changeset: db3706d16f3416ff1d8dfc52c90e692871d2815e

[Alex Buts]

refs #7529 fixing ConvertToMDComponentsTest

Changeset: 4224bad299906ab20741df2049b7c0bdf1d4e720

[Alex Buts]

refs #7529 Doxygen warning

Changeset: 9d6ea35268a88ea3aa0549abf9ad01cf6224fdf0

[Alex Buts]

refs #7529 Added test for 5D and fixed some dependencies.

and invalid properties

Changeset: 827f735d1e1ef646894c99d9249ffc96a0103b09

[Alex Buts]

refs #7529 Changes to WIKI reflecting changes to code

Changeset: 7a4ba269cf3775fb5e160055794d867b580ce15a

[Alex Buts]

refs #7529 Hopefully sorts out issue identified by tester

Changeset: fd70ed899915323388c56d7070e489a7ef8ba9c9

[Alex Buts]

refs #7529 unit test for parameters transformation from ConverToMD

To ConvertToMDHelper

Changeset: e64cd4665dd1206dda3a2654d271998f8c44dc69

[Alex Buts]

refs #7529 Probably this is correct relation

between ConverToMD and ConvertToMDHelper

Changeset: 5fc4397c2fe1c8148bc706cb57141048c9ba9203

[Alex Buts]

refs #7529 fixing unit tests

Changeset: a2ed59c548449b6f0a64360639d0aee45ba94a1e

[Alex Buts]

refs #7529 trying to fix unix error

Changeset: 0b72861b9530607644ef04c3c7c4ce043563d546

[Alex Buts]

refs #7529 Hopefully it fixes it

Changeset: 4b84bef769a02cc114d5c6477a6f534f6775bdc1

[Alex Buts]

refs #7529 Merge conflicts with master

Merge remote-tracking branch 'remotes/origin/master' into feature7529/ConvertToMD_Autolimits

Conflicts:

Code/Mantid/Framework/MDAlgorithms/src/ConvertToMD.cpp Code/Mantid/Framework/MDAlgorithms/test/ConvertToQ3DdETest.h

Changeset: 64db3cd5e9c74fef45f7cbe38900e17166a6238d

[Alex Buts]

refs #7529 Started new ConvertToMDHelper

which would use ConvertToMD core as the main part of the algorithm

Changeset: 8aa4d64c2264138d7f0b443e7e0f3041a260d250

[Alex Buts]

refs #7529 Small further changes

(not in any way completed)

Changeset: ef7371a144b3af63de382387dd01d5041b621ae7

[Alex Buts]

refs #7529 General framework for ConvertToMDHelper2.cpp

probably not yet working correctly

Changeset: 17b287e65ce795a879a13d2bf1daebbdc4c54763

[Alex Buts]

refs #7529 Get rid of test for ConvertToMDHelperParam

as the function has gone

Changeset: 34f464a2f206a2bb779e8f28a6b528bfcecf4857

[Alex Buts]

refs #7529 Enabled "distribution" mode conversion for matrix workspace

Changeset: 239b8d24255464fc77cad9bd2886ccda2627df60

[Alex Buts]

refs #7529 Something working

but may be incorrect and properties are not copied correctly

Changeset: 5bcf30f283adec82afed6f897effccc2423e9a0a

[Alex Buts]

refs #7529 All seems work except some tests

which can be incorrect with new changes.

Changeset: 36747034478d70775ab1d329dcc20544588ab9be

[Alex Buts]

refs #7529 All seems fixed

Changeset: e7cad7115ee911904dda432b84db1ba5585e2917

[Alex Buts]

refs #7529 MAC is different ....

Changeset: 6f51bcb00ffb1e7c73e1cfb938dade55a167ffc6

[Alex Buts]

refs #7529 fixing typo

Changeset: 1ce46356e1cb3616654809cea67cba8f81c119eb

[Alex Buts]

refs #7529 Wiki and comments

Changeset: d52f11c78289b797ad1a176ec49d4b8205be5c07

[Alex Buts]

refs #7529 Minor tidiness

Being a bit more careful instead of thinking.

Changeset: 6150eeeebf93ea74fd975b9e1e69dcd281422dd8

[Alex Buts]

refs #7529 fixing compiler warnings on Nix

Changeset: d026d146601a3234211d49fa88f99586bc2972ed

[Alex Buts]

refs #7529 A bit more wiki description.

Changeset: 66b97b0cd4946d2b3ce983be68c6e13d5fe1de02

[Alex Buts]

refs #7529 Generally fixes the issue with different Units

but more testing is necessary

Changeset: 91cee5ae119178cde5f0b28041427769ca75ab94

[Alex Buts]

refs #7529 further enhancements (not finished)

mainly introducing monotonic ranges method in Units class

Changeset: 7a07304c431dcfbb4bbec97b77d21afda098ddd4

[Alex Buts]

refs #7529 Checkpoint, defining reduction limits

Changeset: 0d24a5076a6a71afdae59a891483cb7bc7427abf

[Alex Buts]

refs #7529 Finished limit for units and unit tests for these limits

Changeset: 207c49d8e6a4ba10c9ac8978f88560d0ea10d27d

[Alex Buts]

refs #7529 removed unnecessary commits

Changeset: 0b14402f0b07a1074416510f108604891c7ab907

[Alex Buts]

refs #7529 Generally fixes the issue with different Units

but more testing is necessary refs #8794 Resolved conflicts while reverting ticket to master

Conflicts:

Code/Mantid/Framework/MDAlgorithms/inc/MantidMDAlgorithms/ConvertToMDHelper2.h Code/Mantid/Framework/MDAlgorithms/src/ConvertToMD.cpp Code/Mantid/Framework/MDAlgorithms/src/ConvertToMDHelper2.cpp

Changeset: 6b3bf28f3b60d10bc00487c27e4c2a2c9ab0df72

[Alex Buts]

refs #8794 mainly introducing monotonic ranges method in Units class

refs #7529 further enhancements (not finished)

Changeset: a540ee9e73b84794bad1c5eb133c70b86c1f5fa4

[Alex Buts]

refs #7529 Checkpoint, defining reduction limits

Changeset: 31e6b9f84ce9209cdd7a4ec7f2a5530cee8ae4a6

[Alex Buts]

refs #7529 Finished limit for units and unit tests for these limits

Changeset: ab818af6809235f57c4ea97dbe906286dc241255

[Alex Buts]

refs #7529 removed unnecessary commits

Changeset: 1c8af68b280ea6eefb93209fc3db0ec562ad6b29

[Alex Buts]

refs #7529 Small further changes

(not in any way completed)

Changeset: 92c6a8e76807ab9d6a1525d096242811e95290ec

[Alex Buts]

refs #7529 Enabled "distribution" mode conversion for matrix workspace

Changeset: 5aca91f8a5aaf26baf6f557b9b6860035e59545b

[Alex Buts]

refs #7529 Something working

but may be incorrect and properties are not copied correctly

Changeset: 1607a579bb1d0a1c80c115c08ba1dd6d0e4a35dd

[Alex Buts]

refs #7529 All seems work except some tests

which can be incorrect with new changes.

Changeset: a2480bfc8a70720ff8e60fd5b63daf6b5c559524

[Alex Buts]

refs #7529 MAC is different ....

Changeset: 2717752a391138f41831425eb8f2ba515d966b42

[Alex Buts]

refs #7529 fixing typo

Changeset: 7140baf670658f192f34c4f2864c2d2d13315f61

[Alex Buts]

This ticket involves so much changes to different parts of the Mantid that it becomes unmanageable.

I've divided it to the range of tickets (all are already fixed and tested) and created ticket #8804 which summarizes and completes the job, started here.

The branch should stay alive for some time in case if somebody wants ConvertToMDHelper V2 have a mode to produce the same results as ConvertToMDHelper V1. The code to do that is already here and should be extracted from this branch.

This branch also contains valuable testing scripts to be used for #8804.

Except this, I am closing this ticket as duplicate.

[Nick Draper]

As suggested in the last comment, this is being closed as a duplicate and therefore NO CODE BRANCHES will be merged.

[Stuart Campbell]

This ticket has been transferred to github issue ​8374