Ticket #9361 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Fix SANS2D IDF and Parameter Name

Reported by: Peter Parker Owned by: Peter Parker
Priority: critical Milestone: Release 3.2
Component: SANS Keywords:
Cc: anders.markvardsen@… Blocked By:
Blocking: Tester: Samuel Jackson

Description (last modified by Peter Parker) (diff)

[EDITED]

For the complete avoidance of doubt, here is a script with which one can test a) which SANS2D Instrument Definition and Parameter files are being used when loading old and new runs, and b) which Parameter files are being loaded when manually loading SANS2D IDFs:

import os

# Set to your instrument directory.
inst_dir = r"C:\Users\stv04463\Development\Mantid\Source\WIP\Code\Mantid\instrument"

OLD_SANS_RUN = "SANS2D00022300.nxs"
NEW_SANS_RUN = "SANS2D00023519.nxs"

IDF_SINCE_9333 = "SANS2D_Tubes_Definition.xml"
PROPOSED_IDF_IN_9361 = "SANS2D_Definition_Tubes.xml"

OLD_IDF = "SANS2D_Definition.xml"
NEW_IDF = IDF_SINCE_9333

def new_idf_was_loaded(inst):
        # A component that existed in the old SANS2D IDF.
        OLD_COMP = "SANS2D/rear-detector/rear-detector(x=184)"
        # A component that exists in the new SANS2D IDF.
        NEW_COMP = "SANS2D/rear-detector/left32"
        
        if inst.getComponentByName(OLD_COMP) != None and inst.getComponentByName(NEW_COMP) == None:
                return False
        if inst.getComponentByName(OLD_COMP) == None and inst.getComponentByName(NEW_COMP) != None:
                return True

def new_param_file_was_loaded(inst):
        # The name of a parameter which should be different in the old and new versions of the param files.
        COL_PARAM = "low-angle-detector-num-columns"
        
        if len(inst.getNumberParameter(COL_PARAM)) == 0:
                return False
        if 192 == inst.getNumberParameter(COL_PARAM)[0]:
                return False
        if 512 == inst.getNumberParameter(COL_PARAM)[0]:
                return True

# a) Load an old and a new run, and see if correct IDFs / param files are being pulled in.
ws = Load(Filename=OLD_SANS_RUN)
inst = ws.getInstrument()
assert not new_idf_was_loaded(inst)
assert not new_param_file_was_loaded(inst)
ws = Load(Filename=NEW_SANS_RUN)
inst = ws.getInstrument()
assert new_idf_was_loaded(inst) # Fails since #9333.
assert new_param_file_was_loaded(inst) # Fails since #9333.

# b) Load the old and new IDFs manually and see if the correct param files are being pulled in.
inst_ws = LoadEmptyInstrument(Filename=os.path.join(inst_dir, OLD_IDF))
inst = inst_ws.getInstrument()
assert not new_idf_was_loaded(inst)
assert not new_param_file_was_loaded(inst)
inst_ws = LoadEmptyInstrument(Filename=os.path.join(inst_dir, NEW_IDF))
inst = inst_ws.getInstrument()
assert new_idf_was_loaded(inst) # Fails since #9333.
assert new_param_file_was_loaded(inst) # Fails since #9333.

Since #9333 the names of the SANS2D IDF and param file have been:

SANS2D_Tubes_Parameters.xml
SANS2D_Tubes_Definition.xml

which makes the script above fail (see comments in script).

Changing to:

SANS2D_Parameters_Tubes.xml
SANS2D_Definition_Tubes.xml

will mean that the script works fine, but that the system tests fail. (This is because the reduction incorrectly pulls in the latest IDF for the instrument rather the the IDF that corresponds to the data being reduced.) This will need to be addressed in this ticket.

Change History

comment:1 Changed 6 years ago by Anders Markvardsen

For reference please note the section "Naming and Using a Parameter File" on http://www.mantidproject.org/InstrumentParameterFile#Naming_and_Using_a_Parameter_File

comment:2 Changed 6 years ago by Peter Parker

  • Description modified (diff)
  • Summary changed from Fix SANS2D IDF Name to Fix SANS2D IDF and Parameter Name

Anders, I misunderstood the naming conventions as documented in that link. I've updated the ticket description accordingly so that we can get things working for SANS2D.

Last edited 6 years ago by Peter Parker (previous) (diff)

comment:3 Changed 6 years ago by Peter Parker

  • Description modified (diff)

comment:4 Changed 6 years ago by Owen Arnold

  • Status changed from new to assigned

Makes sense. Go ahead and work on this defect.

comment:5 Changed 6 years ago by Peter Parker

  • Status changed from assigned to inprogress

Refs #9361 - Rename IDF and param file to match naming conventions.

This ensures that the correct IDF and param file are loaded for old SANS2D files as well as new SANS2D files.

Changeset: 9ef2086b6a70838e8bbcb4837cdf6baec402f39f

comment:6 Changed 6 years ago by Peter Parker

Refs #9361 - Default to standard-named IDF.

In the cases where we're loading IDF's without first loading in the data (this happens for the system tests but not while using the interface under normal circumstances), then default to the IDF with name "INST_Definition.xml" rather than the latest IDF.

This is really just a quick fix so that we can make changes to the new SANS2D IDF and param name that don't break the system tests. The real solution would be to hold off loading any IDF's at all without first knowing which version of the instrument should be used -- this will be done under #9361.

Changeset: 1abb8dd9a27222c75928cc1b8d578e1f30c3b273

comment:7 Changed 6 years ago by Peter Parker

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

comment:8 Changed 6 years ago by Peter Parker

Tester:

  • Test by changing line 13 of the script to read "NEW_IDF = PROPOSED_IDF_IN_9361" and then making sure that the assertions don't fail.
  • Check that the system tests are passing.
Last edited 6 years ago by Peter Parker (previous) (diff)

comment:9 Changed 6 years ago by Samuel Jackson

  • Status changed from verify to verifying
  • Tester set to Samuel Jackson

comment:10 Changed 6 years ago by Samuel Jackson

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/9361_fix_sans2d_idf_name'

Full changeset: 86a620dd6a493af61496f989b51bddb423d1e133

comment:11 Changed 6 years ago by Samuel Jackson

System tests are passing, the changes appear to work as intended and the code changes seem reasonable.

comment:12 Changed 6 years ago by Nick Draper

re #9361 this one should not break the tests

Changeset: 2bb9e3180c9bb15458976d91a396972bfde14188

comment:13 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10204

Note: See TracTickets for help on using tickets.