Ticket #8590 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Refl_gui Improvements - Live Data fix

Reported by: Keith Brown Owned by: Keith Brown
Priority: critical Milestone: Release 3.1
Component: Reflectometry Keywords:
Cc: Blocked By:
Blocking: #7377 Tester: Owen Arnold

Description

Before Owen dealt with it in #8372, quick did something bizarre and bad.

In quick.coadd it used to call startlivedata internally inside quick if the run number was 0 then used the output for processing. Owen has since removed this functionality and it needs put back, but in the right way.

What should happen is that the GUI should be the one to call startlivedata and then pass the resulting workspace to quick as quick can now accept a workspace as a parameter

Change History

comment:1 Changed 7 years ago by Keith Brown

  • Status changed from new to inprogress

Refs #8590. Working on alternate loading functionality

I've created a script that wraps Load and StartLiveData which will invoke the one apropriate for the run number supplied

Changeset: f05ee9e28de6c0f794178b56eda00bec53df48c9

comment:2 Changed 7 years ago by Keith Brown

Refs #8590. Changed the way the check happens

The LoadInstrumentData script has been repurposed as load_live_data and doen't do a zero check when you ask for live data.

Changeset: afd9570efd251437db31c4c05cab26a25c83f255

comment:3 Changed 7 years ago by Keith Brown

Refs #8590 continuing work on integrating startlivedata

The user is now prompted for what Accumulation method they want to use for the live data.

However there is a problem where quick crashed after it tries to get the workspace defualts on the live data workspace. I will need to investigate this.

I plan on merging master into this branch in order to gain the benefit of some of the newer edits to quick, in order to see if they fix this.

Changeset: eaa56fb78c6dfb5c57790bdcd6998abdb22b805f

comment:4 Changed 7 years ago by Keith Brown

Refs #8590. Merge remote-tracking branch 'origin' into feature/8590_Refl_gui_Live_Data_fix

Changeset: 1e9ee5233df2cffdbd33173e81cc39ba3a7225a8

comment:5 Changed 7 years ago by Keith Brown

Refs #8590. Remove debug code

FakeEventDataListener wasn't doing the job as the workspace it generated didn't have an accociated paremeter file so it caused an error later in quick when it tried to get those parameters.

Changeset: 4370b23e1b1e1f972f4ab2835077e7e27ddb865d

comment:6 Changed 7 years ago by Keith Brown

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

This should only be tested by Owen.

This will likely need to be tested in debug as the fake event listeners don't appear in a release build. The wrapper script get_live_data() will also need modifying slightly so that you can force the fake event listeners, otherwise it'll try and connect to an instrument and fail.

Run FakeISISEventDAE and FakeISISHistoDAE to initialise the listeners

Load a tbl file and change the run number of one to 0, then hit process (processing all rows). It should then ask what accumulation mode you require. At this point the Live Data Listener should start and create a workspace. Quick.py will then fail as the fake event listener won't have the appropriate parameters

comment:7 Changed 7 years ago by Keith Brown

  • Tester set to Owen Arnold

comment:8 Changed 7 years ago by Owen Arnold

  • Status changed from verify to verifying

comment:9 Changed 7 years ago by Owen Arnold

No checkbuild! Cannot test it.

comment:10 Changed 7 years ago by Owen Arnold

I can see what you've done here, and I think what you've done should work. However, the following should be addressed within this ticket.

  • Dead code line 371 etc. Dead code should be deleted.
  • Menu option for accumulation method rather than pop-up. Store choice in qtsettings or similar so that it is remembered. Users will definitely get annoyed if they have to select this each time.
  • Add menu option above to documentation (screenshot with help text)
  • Line 520. Replace logic seems unnecessary. Should be able to load via the run number.

Once these things are done, this should be fine to go into master.

comment:11 Changed 7 years ago by Owen Arnold

  • Status changed from verifying to reopened
  • Resolution fixed deleted

comment:12 Changed 7 years ago by Owen Arnold

The decision logic does seem to be working as I have checked that. The other issues will still need to be resolved.

comment:13 Changed 7 years ago by Keith Brown

Update from owen:

Remove dead code

The accumulation method can be saved for another ticket

Ignore the logic change at 520, owen is changing that area anyway

comment:14 Changed 7 years ago by Keith Brown

  • Status changed from reopened to inprogress

Refs #8590 Merge remote-tracking branch 'origin' into feature/8590

Conflicts:

Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py

Changeset: 0e46b72dfffb13f906db7c51ebca79979c88eb35

comment:15 Changed 7 years ago by Keith Brown

Refs #8590 Dead Code removed

Dead code has been removed

Changeset: fa73b4d289e73bd5a6bc7ecf98595480d8c07929

comment:16 Changed 7 years ago by Keith Brown

Owen should really be the one to test this.

To Tester

Make sure the existing functionality works: http://www.mantidproject.org/Testing_ISIS_Reflectometry_GUI

Make sure an attempt to start a live data service for the selected instrument is actually attempted. This attempt should fail as the live data listeners won't be able to connect properly.

The logic is there, and should work provided a connection is available, but we have no way of making sure.

Last edited 7 years ago by Keith Brown (previous) (diff)

comment:17 Changed 7 years ago by Keith Brown

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

comment:18 Changed 7 years ago by Owen Arnold

  • Status changed from verify to verifying

comment:19 Changed 7 years ago by Owen Arnold

Hasn't been fully merged into develop. Once you've done this, I can test it.

comment:20 Changed 7 years ago by Keith Brown

Refs #8590 Merge branch 'feature/8590' into develop

Conflicts:

Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py

Changeset: f09050426d6c23bf9ca4b45cdf9548446399281c

comment:21 Changed 7 years ago by Owen Arnold

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Had to reopen load_live_runs is undefined. Can you please fix this and retest it before adding it back to the testing pool. Thanks, Owen.

comment:22 Changed 7 years ago by Keith Brown

I think that was caused by the movement of modules in your other ticket as the module won't have moved on my branch. I just need to put it in the proper place

comment:23 Changed 7 years ago by Owen Arnold

I've also seen that you're using uppercase parameter names in some of the new functions. Our standard is to only use names starting with upper case to indicate a type. See http://google-styleguide.googlecode.com/svn/trunk/pyguide.html. You can write a separate ticket to clean this up at a later date.

comment:24 Changed 7 years ago by Keith Brown

  • Status changed from reopened to inprogress

refs #8590 Replaced Import Statement

The import had dissapeared, persumably from when owen cleaned some stuff up and git got confused

Changeset: c9b51bc9f9b6e7e201898193399b6e455a532456

comment:25 Changed 7 years ago by Keith Brown

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

comment:26 Changed 7 years ago by Owen Arnold

  • Status changed from verify to closed

Merge remote-tracking branch 'origin/feature/8590_Refl_gui_Live_Data_fix'

Full changeset: 975a4d59d35ed2f9024b349dfb14c7fa127ecbf6

comment:27 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9434

Note: See TracTickets for help on using tickets.