Ticket #7189 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

ConvertToDiffractionMDWorkspace V2 does not properly work in number of system tests

Reported by: Alex Buts Owned by: Alex Buts
Priority: major Milestone: Release 2.6
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Martyn Gigg

Description (last modified by Alex Buts) (diff)

There are two system tests where new ConvertToDiffractionMDWorkspace algorithm does not work properly. These are Diffraction_Workflow_Test and MDWorkspacesTest. To let them running, Version=1 of the algorithm is invoked and the changed rows are marked by word #HACK.

The reason for the differences has to be investigated and fixed if any error found

Change History

comment:1 Changed 7 years ago by Alex Buts

  • Status changed from new to accepted

comment:2 Changed 7 years ago by Alex Buts

SXDAnalysis is also reverted to V1 as it runs substantially slower. Case to investigate why

comment:3 Changed 7 years ago by Alex Buts

refs #7189 version change in place 1 had not done any harm.

Changeset: aa82b2e3e8c8a01d8c3b83f6e81e8aa771fb0efe

comment:4 Changed 7 years ago by Alex Buts

refs #7189 Fixed the Diffraction_Workflow_Test by disabling Lorentz

corrections. There is the questions what should be done about Lorentz corrections, but meanwhile it is the way to make the test pass

Changeset: 2cb44e7b9465feb2144e9295de62bbabb27004ea

comment:5 Changed 7 years ago by Alex Buts

refs #7189 fixed case when target MD workspace does not have

any experiment info attached to it. Hopefully fixed, as further tests are needed for that.

Changeset: a2ff6ea2668ab71e43b10d0416746ce51b0b4177

comment:6 Changed 7 years ago by Alex Buts

refs #7189 Modified peaks intensities and enabled Lorentz corrections

in Diffraction_Workflow_Test. Test passes with new pick intensities, though it still should be investigated

Changeset: 88bd6ed7e9671bc2f36911ae7a915186ad043f99

comment:7 Changed 7 years ago by Alex Buts

refs #7189 Trying to enable MDWorkspaceTests.py

Changeset: b0784629696137c4a591241906d7459bfe9ea5b9

comment:8 Changed 7 years ago by Alex Buts

refs #7189 Fixed goniometer problem for ConvertToMD to work

Changeset: 394c41c7f5bceb4c564c13d20f38bf1d4687bfd9

comment:9 Changed 7 years ago by Alex Buts

refs #7189 Issue clear error message if dimensions units are different

Changeset: 12d6ca332f116a6086d61c368c39d4faa5bdedf7

comment:10 Changed 7 years ago by Alex Buts

refs #7189 ConvertTo MD is able to add lean events to correspondent ws

Problem with convertToMD system test was that the tests were creating MDLeanEventWorkspace and ConvertToMD was trying to add MDEvents to it. Code modified to be able to reduce MDEvents to MDLean events if found correspondent target workspace.

Changeset: f5da95f288f498c3f6ce34ce7012dd7655513938

comment:11 Changed 7 years ago by Nick Draper

  • Status changed from accepted to assigned

comment:12 Changed 7 years ago by Alex Buts

refs #7189 "Fixed" Unit test which verifies adding one type of

workspace to another

It is dirty fix, as because we have allowed adding MDEvents to MDLeanEvents, we have to disable check of the strict workspace types. This solution should be reconsidered after ticket #7473 is completed.

Changeset: 6392763156459b80dc2bb3726428b6c9a9e04a25

comment:13 Changed 7 years ago by Alex Buts

refs #7189 SXDAnalysis changed to version 2

We need to have honest estimate of the algorithm productivity. There is ticket #7228 to fix it if it decreases substantially.

Changeset: 157b3aa51cff606ff373f3bf233d117daf3b5044

comment:14 Changed 7 years ago by Alex Buts

  • Description modified (diff)

To tester:

The ticket needs merging in both Mandid and SystemTests repository.

All system tests which involve ConvertToDiffractionWorkspace have to pass and all calls to ConvertToDiffractionWorkspace should be with default version (no Version=1 in the endm and no #HACK words related to ConvertToDiffractionWorkspace )

There is still suspicious situation with Lorentz corrections referred by ticket #7475 but current agreement is that Version 2 of the algorithm works correctly in all system tests.

It could be substantial loss of performance in some cases, which should be addressed by #7228 and some loss in functionality for ConvertToMD, which should be addressed after #7473

Last edited 7 years ago by Alex Buts (previous) (diff)

comment:15 Changed 7 years ago by Alex Buts

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

comment:16 Changed 7 years ago by Gesner Passos

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:17 Changed 7 years ago by Alex Buts

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

Looking on the test result I can see that it comes from Mantid branch not merged (not compiled properly)

comment:18 Changed 7 years ago by Alex Buts

refs #7189 One #Hack word found with is now irrelevant

Changeset: 894537274c1f5391f0960f817c877207fd230cad

comment:19 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:20 Changed 7 years ago by Martyn Gigg

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

comment:21 Changed 7 years ago by Martyn Gigg

  • Status changed from verifying to reopened
  • Resolution fixed deleted

The SXDAnalysis test is now taking over an hour on the develop systemtests. This is too long. My suspicion is that the raw workspace is going managed. Can we raise the memory threshold of SXDAnalysis to 3Gb while #7228 is being looked at.

comment:22 Changed 7 years ago by Alex Buts

  • Status changed from reopened to inprogress

comment:23 Changed 7 years ago by Alex Buts

refs #7189 modified the memory request to mediate poor performance

of SXDAnalysis test

Changeset: 12ef151aaeb4c5d9677496d45b82fe00795dbd2c

comment:24 Changed 7 years ago by Alex Buts

hopefully fixed the poor performance by increasing the memory requests but need to wait until the system tests pass

comment:25 Changed 7 years ago by Alex Buts

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

comment:26 Changed 7 years ago by Martyn Gigg

  • Status changed from verify to verifying

comment:27 Changed 7 years ago by Martyn Gigg

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/7189_ConvertToMD_V2_inSysTests'

comment:28 Changed 7 years ago by Martyn Gigg

Merge remote-tracking branch 'origin/bugfix/7189_ConvertToMD_V2_inSysTests'

comment:29 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 8035

Note: See TracTickets for help on using tickets.