Transfer PV-logs during load of grouping and mask workspaces

src/snapred/backend/data/util/PV_logs_util.py

@@ -0,0 +1,39 @@
+"""
+Python `Mapping`-interface adapters and utility-methods relating to Mantid workspace logs (i.e. `Run`)
+  and process-variable(PV) logs.
+"""
+# Note: in the upcoming reduction live-data PR this file includes several additional `Mapping` adapters.
+
+from collections.abc import Iterable
+
+from mantid.api import Run
+from mantid.simpleapi import AddSampleLog, mtd
+
+
+def transferInstrumentPVLogs(dest: Run, src: Run, keys: Iterable):
+    # Transfer instrument-specific PV-log values, between the `Run` attributes
+    #   of source and destination workspaces.
+
+    # Placed here for use by various `FetchGroceriesAlgorithm` loaders.
+    for key in keys:
+        if src.hasProperty(key):
+            dest.addProperty(key, src.getProperty(key), True)
+    # REMINDER: the instrument-parameter update still needs to be explicitly triggered!
+
+
+def populateInstrumentParameters(wsName: str):
+    # This utility function is a "stand in" until Mantid PR #38684 can be merged.
+    # (see https://github.com/mantidproject/mantid/pull/38684)
+    # After that, `mtd[wsName].populateInstrumentParameters()` should be used instead.
+
+    # Any PV-log key will do, so long as it is one that always exists in the logs.
+    pvLogKey = "run_title"
+    pvLogValue = mtd[wsName].run().getProperty(pvLogKey).value
+
+    AddSampleLog(
+        Workspace=wsName,
+        LogName=pvLogKey,
+        logText=pvLogValue,
+        logType="String",
+        UpdateInstrumentParameters=True,
+    )

src/snapred/backend/recipe/algorithm/FetchGroceriesAlgorithm.py

@@ -14,6 +14,7 @@
     StringListValidator,
 )
 
+from snapred.backend.data.util.PV_logs_util import populateInstrumentParameters
 from snapred.backend.log.logger import snapredLogger
 from snapred.backend.recipe.algorithm.MantidSnapper import MantidSnapper
 
@@ -147,6 +148,17 @@ def PyExec(self) -> None:
             logger.warning(f"A workspace with name {outWS} already exists in the ADS, and so will not be loaded")
             loaderType = ""
         self.mantidSnapper.executeQueue()
+
+        if loaderType in ["LoadNexus", "LoadEventNexus", "LoadNexusProcessed"]:
+            # TODO: See EWM#7437:
+            #   this clause is necessary to be able to accurately set detector positions
+            #   on files written prior to the merge of the
+            #   `SaveNexus` 'instrument_parameter_map' write-precision fix.
+            # It probably should not be removed, even after that fix is merged.
+            # It should be replaced with `mtd[workspace].updateInstrumentParameters()` after
+            #   Mantid PR#38684 has been merged.
+            populateInstrumentParameters(outWS)
+
         self.setPropertyValue("OutputWorkspace", outWS)
         self.setPropertyValue("LoaderType", str(loaderType))
 

src/snapred/backend/recipe/algorithm/LoadCalibrationWorkspaces.py

@@ -14,8 +14,10 @@
 from mantid.dataobjects import MaskWorkspaceProperty
 from mantid.kernel import Direction
 
+from snapred.backend.data.util.PV_logs_util import populateInstrumentParameters, transferInstrumentPVLogs
 from snapred.backend.log.logger import snapredLogger
 from snapred.backend.recipe.algorithm.MantidSnapper import MantidSnapper
+from snapred.meta.Config import Config
 
 logger = snapredLogger.getLogger(__name__)
 
@@ -30,7 +32,7 @@ class LoadCalibrationWorkspaces(PythonAlgorithm):
         InstrumentDonor: str -- name of the instrument donor workspace
         CalibrationTable: str -- name of the output table workspace
         MaskWorkspace: str -- name of the output mask workspace
-        GroupingWorkspace: str -- name of the output mask workspace
+        GroupingWorkspace: str -- name of the output grouping workspace
     """
 
     def category(self):
@@ -65,7 +67,7 @@ def PyInit(self) -> None:
         )
         self.declareProperty(
             MatrixWorkspaceProperty("GroupingWorkspace", "", Direction.Output, PropertyMode.Optional),
-            doc="Name of the output mask workspace",
+            doc="Name of the output grouping workspace",
         )
 
         self.setRethrows(True)
@@ -135,6 +137,22 @@ def PyExec(self) -> None:
             )
         self.mantidSnapper.executeQueue()
 
+        # Transfer the instrument PV-logs -- not done yet by `LoadDiffCal`.
+        if self.maskWorkspace:
+            transferInstrumentPVLogs(
+                self.mantidSnapper.mtd[self.maskWorkspace].mutableRun(),
+                self.mantidSnapper.mtd[self.getPropertyValue("InstrumentDonor")].run(),
+                Config["instrument.PVLogs.instrumentKeys"],
+            )
+            populateInstrumentParameters(self.maskWorkspace)
+        if self.groupingWorkspace:
+            transferInstrumentPVLogs(
+                self.mantidSnapper.mtd[self.groupingWorkspace].mutableRun(),
+                self.mantidSnapper.mtd[self.getPropertyValue("InstrumentDonor")].run(),
+                Config["instrument.PVLogs.instrumentKeys"],
+            )
+            populateInstrumentParameters(self.groupingWorkspace)
+
         if self.calibrationTable:
             # NOTE ConvertDiffCal has issues ifthe "tofmin" column is present,
             # and LoadFiffCal is written to always add this column on load.

src/snapred/backend/recipe/algorithm/LoadGroupingDefinition.py

@@ -12,6 +12,7 @@
 )
 from mantid.kernel import Direction
 
+from snapred.backend.data.util.PV_logs_util import populateInstrumentParameters, transferInstrumentPVLogs
 from snapred.backend.log.logger import snapredLogger
 from snapred.backend.recipe.algorithm.MantidSnapper import MantidSnapper
 from snapred.meta.Config import Config
@@ -69,7 +70,7 @@ def PyInit(self) -> None:
         )
         self.declareProperty(
             MatrixWorkspaceProperty("InstrumentDonor", "", Direction.Input, PropertyMode.Optional),
-            doc="Workspace to optionally take the instrument from, when GroupingFilename is in XML format",
+            doc="Workspace to optionally take the instrument from",
         )
         self.declareProperty(
             MatrixWorkspaceProperty("OutputWorkspace", "", Direction.Output, PropertyMode.Mandatory),
@@ -145,6 +146,15 @@ def PyExec(self) -> None:
                 InputWorkspace=self.outputWorkspaceName + "_group",
                 OutputWorkspace=self.outputWorkspaceName,
             )
+            self.mantidSnapper.executeQueue()
+            if not self.getProperty("InstrumentDonor").isDefault:
+                # Transfer the instrument PV-logs -- not done yet by `LoadDiffCal`.
+                transferInstrumentPVLogs(
+                    self.mantidSnapper.mtd[self.outputWorkspaceName].mutableRun(),
+                    self.mantidSnapper.mtd[self.getPropertyValue("InstrumentDonor")].run(),
+                    Config["instrument.PVLogs.instrumentKeys"],
+                )
+                populateInstrumentParameters(self.outputWorkspaceName)
         elif self.groupingFileExt in self.supported_xml_file_extensions:
             instrument_donor = self.getPropertyValue("InstrumentDonor")
             preserve_donor = True if instrument_donor else False
@@ -168,12 +178,14 @@ def PyExec(self) -> None:
                     "Deleting instrument definition workspace...",
                     Workspace=instrument_donor,
                 )
+            self.mantidSnapper.executeQueue()
         elif self.groupingFileExt in self.supported_nexus_file_extensions:  # must be a NEXUS file
             self.mantidSnapper.LoadNexusProcessed(
                 "Loading grouping definition from grouping workspace...",
                 Filename=self.groupingFilePath,
                 OutputWorkspace=self.outputWorkspaceName,
             )
+            self.mantidSnapper.executeQueue()
         else:
             raise RuntimeError(
                 f"""
@@ -182,7 +194,6 @@ def PyExec(self) -> None:
                 extensions are {self.all_extensions}
                 """
             )
-        self.mantidSnapper.executeQueue()
         self.setPropertyValue("OutputWorkspace", self.outputWorkspaceName)
 
 

src/snapred/resources/application.yml

@@ -46,6 +46,23 @@ instrument:
     map:
       file: ${instrument.calibration.home}/Powder/LiteGroupMap.hdf
       # file: ${module.root}/resources/ultralite/CRACKLELiteDataMap.xml
+
+  PVLogs:
+    # Swap these when running with ultralite data
+    rootGroup: "entry/DASlogs"
+    # rootGroup: "mantid_workspace_1/logs"
+
+    # PV-log keys relating to instrument settings:
+    instrumentKeys:
+    - "BL3:Chop:Gbl:WavelengthReq"
+    - "BL3:Chop:Skf1:WavelengthUserReq"
+    - "det_arc1"
+    - "det_arc2"
+    - "BL3:Det:TH:BL:Frequency"
+    - "BL3:Mot:OpticsPos:Pos"
+    - "det_lin1"
+    - "det_lin2"
+
   startingRunNumber: 10000
   minimumRunNumber: 46342
   maxNumberOfRuns: 10

tests/integration/test_workflow_panels_happy_path.py

@@ -1,5 +1,8 @@
 import re
 from contextlib import ExitStack, suppress
+
+# TODO: WorkflowNodeComplete signal, at end of each node!
+# Add test-related imports at the end, in order to preserve the import sequence as much as possible.
 from unittest import mock
 
 import pytest
@@ -10,9 +13,11 @@
     QMessageBox,
     QTabWidget,
 )
-from util.pytest_helpers import calibration_home_from_mirror, handleStateInit, reduction_home_from_mirror  # noqa: F401
+from util.pytest_helpers import handleStateInit
 from util.TestSummary import TestSummary
 
+# I would prefer not to access `LocalDataService` within an integration test,
+#   however, for the moment, the reduction-data output relocation fixture is defined in the current file.
 from snapred.meta.Config import Resource
 from snapred.ui.main import SNAPRedGUI, prependDataSearchDirectories
 from snapred.ui.view.DiffCalAssessmentView import DiffCalAssessmentView
@@ -25,8 +30,6 @@
 from snapred.ui.view.reduction.ReductionRequestView import ReductionRequestView
 from snapred.ui.view.reduction.ReductionSaveView import ReductionSaveView
 
-# TODO: WorkflowNodeComplete signal, at end of each node!
-
 
 class InterruptWithBlock(BaseException):
     pass
@@ -494,7 +497,7 @@ def test_calibration_and_reduction_panels_happy_path(
         # Force a clean exit
         qtbot.wait(5000)
 
-    def test_diffraction_calibration_panel_happy_path(self, qtbot, qapp, calibration_home_from_mirror):  # noqa: F811
+    def test_diffraction_calibration_panel_happy_path(self, qtbot, qapp, calibration_home_from_mirror):
         # Override the mirror with a new home directory, omitting any existing
         #   calibration or normalization data.
         tmpCalibrationHomeDirectory = calibration_home_from_mirror()  # noqa: F841

tests/resources/application.yml

@@ -46,6 +46,23 @@ instrument:
       file: ${module.root}/resources/inputs/pixel_grouping/SNAPLite_Definition.xml
     map:
       file: ${instrument.calibration.home}/Powder/LiteGroupMap.hdf
+
+  PVLogs:
+    # Swap these when running with ultralite data
+    rootGroup: "entry/DASlogs"
+    # rootGroup: "mantid_workspace_1/logs"
+
+    # PV-log keys relating to instrument settings:
+    instrumentKeys:
+    - "BL3:Chop:Gbl:WavelengthReq"
+    - "BL3:Chop:Skf1:WavelengthUserReq"
+    - "det_arc1"
+    - "det_arc2"
+    - "BL3:Det:TH:BL:Frequency"
+    - "BL3:Mot:OpticsPos:Pos"
+    - "det_lin1"
+    - "det_lin2"
+
   startingRunNumber: 10000
   minimumRunNumber: 46342
   maxNumberOfRuns: 10

tests/unit/backend/data/test_GroceryService.py

@@ -3,6 +3,8 @@
 import os
 import shutil
 import time
+
+# In order to preserve the import order as much as possible, add test-related imports at the end.
 import unittest
 from pathlib import Path
 from random import randint
@@ -29,8 +31,9 @@
 )
 from mantid.testing import assert_almost_equal as assert_wksp_almost_equal
 from util.Config_helpers import Config_override
+from util.dao import DAOFactory
 from util.helpers import createCompatibleDiffCalTable, createCompatibleMask
-from util.instrument_helpers import mapFromSampleLogs
+from util.instrument_helpers import addInstrumentLogs, getInstrumentLogDescriptors, mapFromSampleLogs
 from util.kernel_helpers import tupleFromQuat, tupleFromV3D
 from util.state_helpers import reduction_root_redirect, state_root_redirect
 from util.WhateversInTheFridge import WhateversInTheFridge
@@ -90,6 +93,16 @@ def setUpClass(cls):
             Filename=cls.instrumentFilePath,
             RewriteSpectraMap=True,
         )
+
+        # Clone a copy of the sample workspace, but without any instrument-parameter state.
+        cls.sampleWSBareInstrument = mtd.unique_hidden_name()
+        CloneWorkspace(OutputWorkspace=cls.sampleWSBareInstrument, InputWorkspace=cls.sampleWS)
+        # Add a complete instrument-parameter state to the sample workspace.
+        # This is now required of the instrument-donor workspace for `LoadGroupingDefinition`
+        #   and `LoadCalibrationWorkspaces`.
+        cls.detectorState = DAOFactory.real_detector_state
+        addInstrumentLogs(cls.sampleWS, **getInstrumentLogDescriptors(cls.detectorState))
+
         SaveNexusProcessed(
             InputWorkspace=cls.sampleWS,
             Filename=cls.sampleWSFilePath,
@@ -841,6 +854,7 @@ def test_fetch_dirty_nexus_native(self):
         # assert the correct workspaces exist
         assert not mtd.doesExist(rawWorkspaceName)
         assert mtd.doesExist(workspaceName)
+
         # test the workspace is correct
         assert_wksp_almost_equal(
             Workspace1=self.sampleWS,
@@ -1445,7 +1459,7 @@ def test_fetch_grocery_list_with_source(self):
     def test_updateInstrumentParameters(self):
         wsName = mtd.unique_hidden_name()
         CloneWorkspace(
-            InputWorkspace=self.sampleWS,
+            InputWorkspace=self.sampleWSBareInstrument,
             OutputWorkspace=wsName,
         )
         assert mtd.doesExist(wsName)