Ticket #8435 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

SANS Redesign Load Data

Reported by: Gesner Passos Owned by: Gesner Passos
Priority: major Milestone: Release 3.1
Component: SANS Keywords:
Cc: anders.markvardsen@…, peter.parker@… Blocked By: #8438
Blocking: #8443 Tester: Peter Parker

Description (last modified by Gesner Passos) (diff)

It is necessary to introduce a clear separation between what is used for processing the reduction (ReductionSteps) and data loaded. Currently, these entities are mixed up, and this introduces problems not only in the design, but also to manipulate these entities.

By breaking these entities we will be able to introduce the changes necessary to process event data multiperiod data asked by larmor.

Change History

comment:1 Changed 7 years ago by Gesner Passos

  • Blocked By 8438 added

comment:2 Changed 7 years ago by Gesner Passos

  • Status changed from new to inprogress
  • Description modified (diff)
  • Summary changed from SANS Redesign LoadSample to SANS Redesign Load Data

comment:3 Changed 7 years ago by Gesner Passos

re #8435: Separate Can and CanSubtraction

now, reducer has a _can_run that works similarly to the _sample_run, and CanSubtraction does only reduce the Can if it is present.

Changeset: 8508f1fd13b939e44b86a0163079c057b9376643

comment:4 Changed 7 years ago by Gesner Passos

re #8435: Adjust usage of background_subtracter inside Mantid

Where it was being used, it is now corrected.

Changeset: 2c97efdb0045dc76847894a93d4e4ab711770e8c

comment:5 Changed 7 years ago by Gesner Passos

re #8435: TransmissionCalc does not use anything from BaseTransmission

Remove the non-sense inheritance.

Changeset: 1738e5ea133d941a4ec6f28eaf49a1ba9ef341c4

comment:6 Changed 7 years ago by Gesner Passos

re #8435: Separate LoadTransmission and TransmissionCalc

One is related to loading data and the other to is a reduction step.

Changeset: c72f2fdd0136a136ecd375bf01a49d7cbb46311f

comment:7 Changed 7 years ago by Gesner Passos

re #8435: Express that loading data is not reduction step

Changeset: b9afd2e7a3a19ee5ff30ae8d39a3b8e041686436

comment:8 Changed 7 years ago by Gesner Passos

re #8435: Impose that AssignSample will be always the first

This commit ensures that the reduction will never use can or transmission from previous time. It will help to the goal of removing the copy and creation of more than one singleton inside the current reduction.

Changeset: 76e8cf244b0bf15cded5d3553aec7569677aeafb

comment:9 Changed 7 years ago by Gesner Passos

re #8435: SANS Reduction

this is a temporary comment to explain why this code still works. It is my intention to get rid of the _setUpPeriod method by applying a better way of ensuring the execution of multiperiod data.

Changeset: 6beb096c9b2e1ca0d92f2022329c99c776f25c64

comment:10 Changed 7 years ago by Gesner Passos

  • Blocking 8443 added

comment:11 Changed 7 years ago by Gesner Passos

re #8435: Fixing UnitTest SansIsisGuiSettings

Changeset: 4ead550ab3a4d8a59d1aa0594660e3216eb1608c

comment:12 Changed 7 years ago by Gesner Passos

re #8435: Ensure geometry correction gets the correct information

Changeset: eb0beb186d1436938cdce23440c2b9e7ff19872e

comment:13 Changed 7 years ago by Gesner Passos

re #8435: SANSReduction requires passing first AssignSample

The GUI does this, so, it does not break the GUI rule.

Changeset: https://github.com/mantidproject/systemtests/commit/652a4d69474bf237716bcd313c059973769026d7

Last edited 7 years ago by Gesner Passos (previous) (diff)

comment:14 Changed 7 years ago by Gesner Passos

Tester:

Before explaining how to test, I want to explain the reasons and what we will gain by having these changes.

  • By providing a separation of that is data and what is reduction steps we will be able to pursue the objective placed in #8443 and #8444.
  • It is also a question of design improvement. Do not mix-up entities.
  • The steps now ask the reducer in execution time for the inputs, and this will allow to reuse the steps without recreating the singleton.
  • There is two changes I'm not really happy with:
    • Why we can not execute the geometry steps for the CAN and we can not correct the geometry of the CAN using its own geometry. (comment:12). I've asked the scientits if this is correct. """The scientist confirmed it is correct."""
  • I was forced to update the systemtest as well comment:13, because it was assigning Sample after transmission, which is not allowed anymore. This in fact will ensure that all the reductions will not be influenced by data of previous reduction.
Last edited 7 years ago by Gesner Passos (previous) (diff)

comment:15 Changed 7 years ago by Gesner Passos

Remember of update code and systemtest.

  • Open ISIS
  • load systemtests/Data/SANS2D/MaskSANS2DReductionGUI.txt
  • change to batch mode
  • load sans2d_reduction_gui_batch.csv
  • perform the reduction
  • change to single mode
  • perform the reduction
  • check that they are the same as systemtests/SystemTests/AnalysisTests/ReferenceResults/SANSReductionGUI.nxs
  • If you type CheckWorkspacesMatch it may complain that the sample name is not the same, because one is and the other ' '. I've already raised a ticket for this.

comment:16 Changed 7 years ago by Gesner Passos

I could not forgive myself to break an agreement and ship a bad code (comment:12). - I broke the agreement that variables starting by underscore is meant to be protected or private. Today I thought a new way of implementing it, and releasing my conscience. Besides, it does improve a little bit the performance by not calculating the same value twice.

comment:17 Changed 7 years ago by Gesner Passos

The existence of many singletons does not allow me to solve the issue presented in comment:16 as I planed #8455. I will have to wait to get the #8443 sorted before.

comment:18 Changed 7 years ago by Gesner Passos

re #8435: SANSReduction requires passing first AssignSample

The GUI does this, so, it does not break the GUI rule.

Changeset: 38991892e6366623291ec2c31d2d50c1b3a095d5

comment:19 Changed 7 years ago by Gesner Passos

  • Status changed from inprogress to verify
  • Cc anders.markvardsen@…, peter.parker@… added
  • Resolution set to fixed

comment:20 Changed 7 years ago by Gesner Passos

It is probably better to let Anders or Peter Parker to test this ticket.

Last edited 7 years ago by Gesner Passos (previous) (diff)

comment:21 Changed 7 years ago by Peter Parker

  • Status changed from verify to verifying
  • Tester set to Peter Parker

comment:22 Changed 7 years ago by Peter Parker

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/8435_redesign_load_data'

Full changeset: 9f24f65f00b0603c426fbeca35b8bb7c10154ffa

comment:23 Changed 7 years ago by Peter Parker

Merge remote-tracking branch 'origin/feature/8435_redesign_load_data'

Full changeset: a53daa3c28d91a003c366ff5a52d52e4063b88f1

comment:24 Changed 7 years ago by Gesner Passos

Merge branch 'feature/8435_redesign_load_data' into feature/8443_many_singletons

Full changeset: 519339a03b2f131c871fe3805267acde57871d25

comment:25 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9279

Note: See TracTickets for help on using tickets.