Ticket #8026 (closed)

Opened 7 years ago

Last modified 5 years ago

ConvertToMD fails when it should not

Reported by: Andrei Savici Owned by: Alex Buts
Priority: critical Milestone: Release 3.2
Component: Framework Keywords:
Cc: Blocked By: #8804, #9396
Blocking: #9502 Tester: Martyn Gigg

Description

Get a workspace that you can convert to |Q|. It is important not to have UB matrix or goniometer on it. From Mantidplot, open the ConvertToMD dialog, select QDimensions=Q3D, Q3DFrames=HKL, QConversionScales=HKL (this might be what were the values from when I previously run the algorithm). No need to execute before, just change QDimensions to |Q|, and try to run it. I get

 Error in execution of algorithm ConvertToMD:
 HKL frame needs UB matrix defined on the workspace

If I choose |Q|, it should not check for UB matrix. Works If I go back to Q3D, choose Q3DFrames=AutoSelect, then go back to |Q|

Change History

comment:1 Changed 7 years ago by Alex Buts

Q mode uses rotation matrix to convert to Q and transform a coordinate system (e.g HKL or A-1) but then calculates Q2. This is also necessary for adding two images with different crystal orientation in Q-mode.

I suspect the bug is in GUI which hides Q-frame and Q-scale (or Q-scale only) setting from the algorithm but can not be entirely sure.

comment:2 Changed 7 years ago by Alex Buts

  • Milestone changed from Release 3.0 to Backlog

comment:3 Changed 7 years ago by Alex Buts

  • Milestone changed from Backlog to Release 3.1

comment:4 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 3.1 to Backlog

Moved to backlog at the end of Release 3.1

comment:5 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move to assigned at the introduction of the triage step

comment:6 Changed 7 years ago by Alex Buts

  • Blocked By 8804 added

Convert to MD has been changed in #8804 so the block to avoid merge conflicts

comment:7 Changed 6 years ago by Alex Buts

  • Status changed from assigned to inprogress
  • Milestone changed from Backlog to Release 3.2

comment:8 Changed 6 years ago by Alex Buts

refs #8026 Renamed ConvertToMDHelper and ConvertToMDHelper2

into ConvertToMDMinMaxGlobal, and ConvertToMDMinMaxLocal

Changeset: 9cb21ba9bc5e8b619f8de8a5e4615149bd13917d

comment:9 Changed 6 years ago by Alex Buts

  • Blocked By 9396 added

(In #9396) This ticket renames algorithms without changes in functionality.

It is easy to test as changes are trivial.

comment:10 Changed 6 years ago by Alex Buts

refs #8026 Unit test illustrating the problem

and some spelling errors.

Changeset: 84dabdd7ca2f9cde32f29fc70912f50ba356072e

comment:11 Changed 6 years ago by Alex Buts

refs #8026 Partial fix which breaks number of tests

and some spelling errors.

Changeset: 9f57e008d97f9dfd45065cea67b6000c7645db52

comment:12 Changed 6 years ago by Alex Buts

refs #8026 Redefined isPowder function to check transformation mode

and more spelling errors.

Changeset: fe158e3dd0e005ca4e8e7336404f700af72d9a3d

comment:13 Changed 6 years ago by Alex Buts

The sequence described in the ticket should work now.

Test only after #9396 is merged into master as I've started from this ticket to avoid merge conflicts.

comment:14 Changed 6 years ago by Alex Buts

  • Blocked By 9396 removed

comment:14 Changed 6 years ago by Alex Buts

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

comment:16 Changed 6 years ago by Alex Buts

refs #8026 minor comments

Changeset: c4bf985c0831f57b383595508b4357f78becd22c

comment:17 Changed 6 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester set to Nick Draper

comment:18 Changed 6 years ago by Nick Draper

  • Status changed from verifying to reopened
  • Resolution fixed deleted
  • Tester Nick Draper deleted

This needs a section on how to test, ideally with a sample python script

comment:19 Changed 6 years ago by Alex Buts

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

it is the bugfix and the problem was that wrong parameters were stuck in GUI. The ultimate test is that the problem described in the ticket does not appear any more.

Tester should try the sequence described in the ticket before applying the patch to confirm that the problem was existing and then try the same sequence after applying the patch. No failure should be observed.

comment:20 Changed 6 years ago by Alex Buts

first commit to the ranch ticket was actually to ticket #9396. I mistyped the ticket number but the code changes are in the basis brunch, not in this one (and are already tested/merged).

comment:21 Changed 6 years ago by Alex Buts

refs #8026 Merge branch 'bugfix/8026_ConvertToMDFails' into develop

Conflicts:

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

Changeset: 74debd3b888682bf845c601e40049cf57e5f1868

comment:22 Changed 6 years ago by Alex Buts

  • Blocking 9502 added

(In #9502) 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:23 Changed 6 years ago by Nick Draper

  • Status changed from verify to reopened
  • Resolution fixed deleted

This brach has a merge conflict on ConvertToMD.cpp.

Please resolve and resubmit.

comment:24 Changed 6 years ago by Martyn Gigg

Using kdfiff the conflict here seems to be entirely because of the wiki section removal - everything else merges just fine. I'm going to pick this back up for testing and check it.

comment:25 Changed 6 years ago by Martyn Gigg

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

comment:26 Changed 6 years ago by Martyn Gigg

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/8026_ConvertToMDFails'

Conflicts:

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

Full changeset: 266e0563595adc4266390d520d0a688d3742ac09

comment:27 Changed 6 years ago by Alex Buts

refs #8026 Merge branch 'bugfix/8026_ConvertToMDFails' into develop

Conflicts:

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

Changeset: 74debd3b888682bf845c601e40049cf57e5f1868

comment:28 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 8871

Note: See TracTickets for help on using tickets.