Ticket #10594 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Error in old refl gui when processing polref runs

Reported by: Harry Jeffery Owned by: Harry Jeffery
Priority: blocker Milestone: Release 3.3
Component: Reflectometry Keywords:
Cc: Blocked By:
Blocking: Tester: Federico M Pouzols

Description

From Max: Just tried some Polref data (run 12379 with 12 378 as transmission run) and got the following:

Traceback (most recent call last):
File "C:/MantidInstall/scripts/Interface\ui\reflectometer\refl_gui.py", line 745, in _process
theta, qmin, qmax, wlam, wq = self._do_run(runno[i], row, i)
File "C:/MantidInstall/scripts/Interface\ui\reflectometer\refl_gui.py", line 931, in _do_run
if mtd.doesExist(transrun_named) and mtd[transrun_named].getAxis(0).getUnit().unitID() == "Wavelength":
AttributeError: 'WorkspaceGroup' object has no attribute 'getAxis'
Traceback (most recent call last):
File "C:/MantidInstall/scripts/Interface\ui\reflectometer\refl_gui.py", line 745, in _process
theta, qmin, qmax, wlam, wq = self._do_run(runno[i], row, i)
File "C:/MantidInstall/scripts/Interface\ui\reflectometer\refl_gui.py", line 987, in _do_run
inst = wq.getInstrument()
AttributeError: 'WorkspaceGroup' object has no attribute 'getInstrument'

Attachments

spectrum4_old_gui.png (19.8 KB) - added by Federico M Pouzols 6 years ago.
Spectrum 4 with old GUI
spectrum4_new_gui.png (14.7 KB) - added by Federico M Pouzols 6 years ago.
Spectrum 4 with new GUI
old_gui_logarithmic.png (36.9 KB) - added by Harry Jeffery 6 years ago.

Change History

comment:1 Changed 6 years ago by Harry Jeffery

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

comment:3 Changed 6 years ago by Harry Jeffery

Refs #10594 Fix workspace group access

Changeset: e1758eab054b31a15a321a14e36e0c9c84bc212a

comment:4 Changed 6 years ago by Harry Jeffery

Refs #10594 Unroll algorithm execution on groups

Changeset: dbbbbbfe3ccbf1a80b823653d59b2aa4cfcfdcab

comment:5 Changed 6 years ago by Harry Jeffery

Refs #10594 Handle WorkspaceGroups better

Changeset: eca9e5a364a8d0fd121e827b0e43f89a60977d6d

comment:6 Changed 6 years ago by Harry Jeffery

Refs #10594 Handle group workspaces in new Refl UI

Changeset: 416accb0faeaf154e5334aef6dfeb7810f3472bd

comment:7 Changed 6 years ago by Harry Jeffery

Refs #10594 Fall back on ReflectometryReductionOne to calculate theta

Changeset: 64ebf9725b79344072c2ed155ba4eb3f47220c8f

comment:8 Changed 6 years ago by Harry Jeffery

Refs #10594 Correct handling of transmission runs

Changeset: 958d2af983591d1985f3696a4bb7e06da588eb10

comment:9 Changed 6 years ago by Owen Arnold

I've had a deep look into this. Here are things that should be checked and fixed as part of this ticket.

0) With the Branch in it's current state. The mantid doesn't shutdown properly. If you get to the point that CalculateResolution is complaining about not being able to calculate dq/q. It looks like CalculateResolution hasn't completed according to the algorithm status, and it isn't allowing me to close Mantid without forcing it. This may be a problem around group processing. See (1) before looking for a fix to this, as your fix for (1) may fix this too.

1) CalculateResolution is processing a group and returning a double param in the same way that ReflectometryReductionOne was. Make this more robust around group processing.

2) Length of angle check in refl_gui is bad. What if i wanted to just enter an integer say 1. see line 947.

3) When looking for theta log values. The gui should check for stheta, as well as theta. And if those are not found, look for twotheta and divide by two. This should be coded nicely of course, pehapse a list of possible a map of values to query as well as a transformation, say *0.5 or *1.

4) I'm not sure whether caching results in the TOF group is a good idea from group workspaces like the POLREF ones you have been using. Best case is that they just get reloaded again because groups can only exist at one level. Please check.

5) Both GUIs should work the same and produce the same results, but for the moment, the dq/q will HAVE to be specified owning to another error (see below).


Other problems I found. Not for us to fix.

6) The SpecularReflectionCalculateTheta is failing because it relies on the point detector and sample positions being set correctly. This is done well for inter by using log values to specify the z-position of the point detector. However POLREFS idf is broken in this regard. It refers to the theta log, which never gets written, so the offset in z is zero, and theta is therefore calculated to be zero (which is why everything else breaks).

7) Even if you provide theta, CalculateResolution does not work, because the beam divergence equation relies of there being a vertical offset between the slits. This instrument parameter does not exist for POLREF, and I don't know what log parameter to hook it up to.

Last edited 6 years ago by Owen Arnold (previous) (diff)

comment:10 Changed 6 years ago by Harry Jeffery

Refs #10594 Fix angle length check in refl_gui

Changeset: 40770ac050c2255f9d713bdbf1f2d3d415d5de26

comment:11 Changed 6 years ago by Harry Jeffery

Refs #10594 refl_gui should not give CalcRes workspace groups

Changeset: 9f4b306ff95d8410ccd812529728ff93e10e67c6

comment:12 Changed 6 years ago by Harry Jeffery

Refs #10594 Don't put group workspaces in a TOF group

Changeset: 07617162b2eb513377abb4eb24d289d0712a70b5

comment:13 Changed 6 years ago by Harry Jeffery

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

Testing

In both the old and new UI:

  • Attempt to process sample run POLREF00012379 with transmission run POLREF0001378.
    • It will fail. Theta cannot be automatically determined, nor can the resolution (dQ/Q).
  • Enter a value for dQ/Q (0.04 is sensible).
  • Attempt to process again
    • It will succeed, but Theta will be calculated as 0.
      • 0 is incorrect, but for reasons unrelated to this ticket
  • Enter a value for theta of your own choosing (0.1 -> 2.5 is a sensible range)
  • Attempt to process again
    • It should succeed

To reproduce the never-ending CalculateResolution execution, in the old UI:

  • Attempt to process sample run POLREF00012379 with transmission run POLREF0001378, and theta = 0.7

To confirm both UI's produce the same output:

  • In the old UI process sample run POLREF00012379 with transmission run POLREF0001378, theta = 0.7, and resolution = 0.04
  • Rename 12349_IvsQ to old_ivsq
  • Delete all other workspaces
  • In the new UI process sample run POLREF00012379 with transmission run POLREF0001378, theta = 0.7, and resolution = 0.04
  • Plot spectra 4 of IvsQ_12349 and old_ivsq together. They should be identical.
    • Beware that there will be two lines visible, one for each period, but both workspaces should have the same two periods each.

comment:14 Changed 6 years ago by Federico M Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico M Pouzols

Changed 6 years ago by Federico M Pouzols

Spectrum 4 with old GUI

Changed 6 years ago by Federico M Pouzols

Spectrum 4 with new GUI

comment:15 Changed 6 years ago by Federico M Pouzols

I followed the instructions and I got the expected errors. But in the end I don't get the same results with the old and new GUIs. When I set dQ/q to 0.04 and angle to 0.5 what I get from PlotSpectrum (spectrum 4 of the IvsQ workspace) is the attached plots. Maybe I'm missing a step or parameter?

comment:16 Changed 6 years ago by Harry Jeffery

Looks like you may have missed the transmission run when processing in the old gui.

Also, it's worth right-clicking on the plot, and selecting Axes->Log(x),Log(y), as the results are logarithmic they'll show up much better that way. They should look like old_gui_logarithmic.png

Changed 6 years ago by Harry Jeffery

comment:17 Changed 6 years ago by Federico M Pouzols

  • Status changed from verifying to closed

Oh sorry, I was putting the transmission run number in the wrong place! Now they look identical.

comment:18 Changed 6 years ago by Federico Montesino Pouzols

Merge remote-tracking branch 'origin/feature/10594_refl_gui_processing_polref_error'

Full changeset: d289cb7b7cb96d910afdf85c2432a1ae7d6480e4

comment:19 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11436

Note: See TracTickets for help on using tickets.