Simple fix to Defect 8554 (#526)

* service always loads diffcal mask, use correct name

* add hnew helpers, new test of diffcal mask only

* changes from code review

* simplify maskFromArray

* remove

* use preexisting add sample log helper

* fixup! use preexisting add sample log helper

src/snapred/backend/dao/state/DetectorState.py

@@ -1,6 +1,6 @@
 from enum import IntEnum
 from numbers import Number
-from typing import Literal, Tuple
+from typing import Dict, Literal, Tuple
 
 from pydantic import BaseModel, field_validator
 
@@ -26,3 +26,24 @@ def validate_int(cls, v):
             #   e.g. hdf5 returns `int64`
             v = int(v)
         return v
+
+    @classmethod
+    def constructFromLogValues(cls, logValues):
+        return DetectorState(
+            arc=(float(logValues["det_arc1"]), float(logValues["det_arc2"])),
+            lin=(float(logValues["det_lin1"]), float(logValues["det_lin2"])),
+            wav=float(logValues["BL3:Chop:Skf1:WavelengthUserReq"]),
+            freq=float(logValues["BL3:Det:TH:BL:Frequency"]),
+            guideStat=int(logValues["BL3:Mot:OpticsPos:Pos"]),
+        )
+
+    def getLogValues(self) -> Dict[str, str]:
+        return {
+            "det_lin1": str(self.lin[0]),
+            "det_lin2": str(self.lin[1]),
+            "det_arc1": str(self.arc[0]),
+            "det_arc2": str(self.arc[1]),
+            "BL3:Chop:Skf1:WavelengthUserReq": str(self.wav),
+            "BL3:Det:TH:BL:Frequency": str(self.freq),
+            "BL3:Mot:OpticsPos:Pos": str(self.guideStat),
+        }

src/snapred/backend/data/GroceryService.py

@@ -513,6 +513,27 @@ def setWorkspaceTag(self, workspaceName: str, logname: str, logvalue: str):
         else:
             raise RuntimeError(f"Workspace {workspaceName} does not exist")
 
+    def checkPixelMask(self, pixelMask: WorkspaceName) -> bool:
+        """
+        Check if a pixel mask is valid.
+        :param pixelMask: the name of the MaskWorkspace to check
+        :type pixelMask: Workspacename
+        :return: True if pixelMask is a non-trivial MaskWorkspace in the ADS.  False otherwise.
+        :rtype: bool
+        """
+        # A non-trivial mask is a mask that has a non-zero number of masked pixels.
+
+        if not self.mantidSnapper.mtd.doesExist(pixelMask):
+            return False
+        else:
+            mask = self.mantidSnapper.mtd[pixelMask]
+            if not isinstance(mask, MaskWorkspace):
+                return False
+            elif mask.getNumberMasked() == 0:
+                return False
+            else:
+                return True
+
     ## FETCH METHODS
     """
     The fetch methods orchestrate finding data files, loading them into workspaces,

src/snapred/backend/recipe/ReductionRecipe.py

@@ -63,7 +63,7 @@ def unbagGroceries(self, groceries: Dict[str, Any]):
         self.groceries = groceries.copy()
         self.sampleWs = groceries["inputWorkspace"]
         self.normalizationWs = groceries.get("normalizationWorkspace", "")
-        self.maskWs = groceries.get("combinedMask", "")
+        self.maskWs = groceries.get("combinedPixelMask", "")
         self.groupingWorkspaces = groceries["groupingWorkspaces"]
 
     def _cloneWorkspace(self, inputWorkspace: str, outputWorkspace: str) -> str:

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

@@ -7,6 +7,7 @@
     PythonAlgorithm,
     WorkspaceUnitValidator,
 )
+from mantid.dataobjects import GroupingWorkspace
 from mantid.kernel import Direction, StringMandatoryValidator
 
 from snapred.backend.dao.state.PixelGroup import PixelGroup as Ingredients
@@ -78,7 +79,7 @@ def validateInputs(self) -> Dict[str, str]:
         # make sure the input workspace can be reduced by this grouping workspace
         inWS = self.mantidSnapper.mtd[self.getPropertyValue("InputWorkspace")]
         groupWS = self.mantidSnapper.mtd[self.getPropertyValue("GroupingWorkspace")]
-        if "Grouping" not in groupWS.id():
+        if not isinstance(groupWS, GroupingWorkspace):
             errors["GroupingWorkspace"] = "Grouping workspace must be an actual GroupingWorkspace"
         elif inWS.getNumberHistograms() == len(groupWS.getGroupIDs()):
             msg = "The data appears to have already been diffraction focused"

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

@@ -7,6 +7,7 @@
     PythonAlgorithm,
     mtd,
 )
+from mantid.dataobjects import GroupingWorkspace
 from mantid.kernel import Direction, ULongLongPropertyWithValue
 
 from snapred.meta.pointer import create_pointer
@@ -24,7 +25,7 @@ def category(self):
     def validateInputs(self) -> Dict[str, str]:
         err = {}
         focusWS = mtd[self.getPropertyValue("GroupingWorkspace")]
-        if focusWS.id() != "GroupingWorkspace":
+        if not isinstance(focusWS, GroupingWorkspace):
             err["GroupingWorkspace"] = "The workspace must be a GroupingWorkspace"
         return err
 

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

@@ -1,6 +1,7 @@
 from typing import Dict, List
 
 from mantid.api import (
+    IEventWorkspace,
     IEventWorkspaceProperty,
     MatrixWorkspaceProperty,
     PropertyMode,
@@ -94,7 +95,7 @@ def unbagGroceries(self):
         self.inputWorkspaceName = self.getPropertyValue("InputWorkspace")
         self.outputWorkspaceName = self.getPropertyValue("OutputWorkspace")
         self.focusWorkspace = self.getPropertyValue("GroupingWorkspace")
-        self.isEventWs = mtd[self.inputWorkspaceName].id() == "EventWorkspace"
+        self.isEventWs = isinstance(mtd[self.inputWorkspaceName], IEventWorkspace)
 
     def PyExec(self):
         """

src/snapred/backend/service/ReductionService.py

@@ -1,5 +1,4 @@
 import json
-from collections.abc import Iterable
 from pathlib import Path
 from typing import Any, Dict, List, Optional
 
@@ -274,9 +273,7 @@ def loadAllGroupings(self, runNumber: str, useLiteMode: bool) -> Dict[str, Any]:
         }
 
     # WARNING: `WorkspaceName` does not work with `@FromString`!
-    def prepCombinedMask(
-        self, runNumber: str, useLiteMode: bool, timestamp: float, pixelMasks: Iterable[WorkspaceName]
-    ) -> WorkspaceName:
+    def prepCombinedMask(self, request: ReductionRequest) -> WorkspaceName:
         """
         Combine all of the individual pixel masks for application and final output
         """
@@ -289,9 +286,42 @@ def prepCombinedMask(
             ==> TO / FROM mask-dropdown in Reduction panel
             This MUST be a list of valid `WorkspaceName` (i.e. containing their original `builder` attribute)
         """
+        runNumber = request.runNumber
+        useLiteMode = request.useLiteMode
+        timestamp = request.timestamp
         combinedMask = wng.reductionPixelMask().runNumber(runNumber).timestamp(timestamp).build()
+
+        # if there is a mask associated with the diffcal file, load it here
+        calVersion = None
+        if ContinueWarning.Type.MISSING_DIFFRACTION_CALIBRATION not in request.continueFlags:
+            calVersion = self.dataFactoryService.getLatestApplicableCalibrationVersion(runNumber, useLiteMode)
+        if calVersion is not None:  # WARNING: version may be _zero_!
+            self.groceryClerk.name("diffcalMaskWorkspace").diffcal_mask(runNumber, calVersion).useLiteMode(
+                useLiteMode
+            ).add()
+
+        # if the user specified masks to use, also pull those
+        residentMasks = {}
+        for mask in request.pixelMasks:
+            match mask.tokens("workspaceType"):
+                case wngt.REDUCTION_PIXEL_MASK:
+                    runNumber, temp_ts = mask.tokens("runNumber", "timestamp")
+                    self.groceryClerk.name(mask).reduction_pixel_mask(runNumber, temp_ts).useLiteMode(useLiteMode).add()
+                case wngt.REDUCTION_USER_PIXEL_MASK:
+                    numberTag = mask.tokens("numberTag")
+                    residentMasks[mask] = wng.reductionUserPixelMask().numberTag(numberTag).build()
+                case _:
+                    raise RuntimeError(
+                        f"reduction pixel mask '{mask}' has unexpected workspace-type '{mask.tokens('workspaceType')}'"  # noqa: E501
+                    )
+        # Load all pixel masks
+        allMasks = self.groceryService.fetchGroceryDict(
+            self.groceryClerk.buildDict(),
+            **residentMasks,
+        )
+
         self.groceryService.fetchCompatiblePixelMask(combinedMask, runNumber, useLiteMode)
-        for n, mask in enumerate(pixelMasks):
+        for n, mask in enumerate(allMasks.values()):
             self.mantidSnapper.BinaryOperateMasks(
                 f"combine from pixel mask {n}...",
                 InputWorkspace1=combinedMask,
@@ -369,37 +399,10 @@ def fetchReductionGroceries(self, request: ReductionRequest) -> Dict[str, Any]:
                 request.runNumber, request.useLiteMode
             )
 
-        # Fetch pixel masks
-        residentMasks = {}
-        combinedPixelMask = None
-        if request.pixelMasks:
-            for mask in request.pixelMasks:
-                match mask.tokens("workspaceType"):
-                    case wngt.REDUCTION_PIXEL_MASK:
-                        runNumber, timestamp = mask.tokens("runNumber", "timestamp")
-                        self.groceryClerk.name(mask).reduction_pixel_mask(runNumber, timestamp).useLiteMode(
-                            request.useLiteMode
-                        ).add()
-                    case wngt.REDUCTION_USER_PIXEL_MASK:
-                        numberTag = mask.tokens("numberTag")
-                        residentMasks[mask] = wng.reductionUserPixelMask().numberTag(numberTag).build()
-                    case _:
-                        raise RuntimeError(
-                            f"reduction pixel mask '{mask}' has unexpected workspace-type '{mask.tokens('workspaceType')}'"  # noqa: E501
-                        )
-            if calVersion is not None:  # WARNING: version may be _zero_!
-                self.groceryClerk.name("diffcalMaskWorkspace").diffcal_mask(request.runNumber, calVersion).useLiteMode(
-                    request.useLiteMode
-                ).add()
-            # Load any non-resident pixel masks
-            maskGroceries = self.groceryService.fetchGroceryDict(
-                self.groceryClerk.buildDict(),
-                **residentMasks,
-            )
-            # combine all of the pixel masks, for application and final output
-            combinedPixelMask = self.prepCombinedMask(
-                request.runNumber, request.useLiteMode, request.timestamp, maskGroceries.values()
-            )
+        # Fetch pixel masks -- if nothing is masked, nullify
+        combinedPixelMask = self.prepCombinedMask(request)
+        if not self.groceryService.checkPixelMask(combinedPixelMask):
+            combinedPixelMask = None
 
         # gather the input workspace and the diffcal table
         self.groceryClerk.name("inputWorkspace").neutron(request.runNumber).useLiteMode(request.useLiteMode).add()
@@ -415,8 +418,8 @@ def fetchReductionGroceries(self, request: ReductionRequest) -> Dict[str, Any]:
             ).add()
 
         groceries = self.groceryService.fetchGroceryDict(
-            groceryDict=self.groceryClerk.buildDict(),
-            **({"combinedPixelMask": combinedPixelMask} if combinedPixelMask else {}),
+            self.groceryClerk.buildDict(),
+            **({"combinedPixelMask": combinedPixelMask} if bool(combinedPixelMask) else {}),
         )
 
         self._markWorkspaceMetadata(request, groceries["inputWorkspace"])

tests/unit/backend/dao/test_DetectorState.py

@@ -0,0 +1,26 @@
+# note: this runs the same checks as the calibrant_samples_script CIS test
+import unittest
+
+from snapred.backend.dao.state.DetectorState import DetectorState
+
+
+class TestDetectorstate(unittest.TestCase):
+    def test_getLogValues(self):
+        exp = {
+            "det_lin1": "1.0",
+            "det_lin2": "1.1",
+            "det_arc1": "2.2",
+            "det_arc2": "2.3",
+            "BL3:Chop:Skf1:WavelengthUserReq": "3.4",
+            "BL3:Det:TH:BL:Frequency": "4.5",
+            "BL3:Mot:OpticsPos:Pos": "2",
+        }
+        detectorState = DetectorState.constructFromLogValues(exp)
+        assert detectorState.arc[0] == float(exp["det_arc1"])
+        assert detectorState.arc[1] == float(exp["det_arc2"])
+        assert detectorState.lin[0] == float(exp["det_lin1"])
+        assert detectorState.lin[1] == float(exp["det_lin2"])
+        assert detectorState.wav == float(exp["BL3:Chop:Skf1:WavelengthUserReq"])
+        assert detectorState.freq == float(exp["BL3:Det:TH:BL:Frequency"])
+        assert detectorState.guideStat == float(exp["BL3:Mot:OpticsPos:Pos"])
+        assert exp == detectorState.getLogValues()

tests/unit/backend/data/test_GroceryService.py

@@ -13,12 +13,15 @@
 from mantid.kernel import V3D, Quat
 from mantid.simpleapi import (
     CloneWorkspace,
+    CreateSampleWorkspace,
     CreateWorkspace,
     DeleteWorkspace,
+    ExtractMask,
     GenerateTableWorkspaceFromListOfDict,
     GroupWorkspaces,
     LoadEmptyInstrument,
     LoadInstrument,
+    MaskDetectors,
     SaveDiffCal,
     SaveNexus,
     SaveNexusProcessed,
@@ -174,7 +177,7 @@ def mockIndexer(self, root=None, calType=None):
     def clearoutWorkspaces(self) -> None:
         """Delete the workspaces created by loading"""
         for ws in mtd.getObjectNames():
-            if ws not in self.excludeAtTeardown:
+            if ws not in self.excludeAtTeardown and ws in mtd:
                 DeleteWorkspace(ws)
 
     def tearDown(self):
@@ -190,7 +193,8 @@ def tearDownClass(cls):
         and remove the test file.
         """
         for ws in mtd.getObjectNames():
-            DeleteWorkspace(ws)
+            if ws in mtd:
+                DeleteWorkspace(ws)
         os.remove(cls.sampleWSFilePath)
         os.remove(cls.sampleDiffCalFilePath)
         os.remove(cls.sampleTarWsFilePath)
@@ -1893,3 +1897,35 @@ def test_lookupDiffcalTableWorkspaceName_missing_workspace_in_record(self):
 
         with pytest.raises(RuntimeError, match=r".*Could not find diffcal table in record*"):
             self.instance.lookupDiffcalTableWorkspaceName(runNumber, True, version)
+
+    def test_checkPixelMask(self):
+        # raises an error if workspace not in ADS
+        nonexistent = mtd.unique_name(prefix="_mask_check_")
+        assert not mtd.doesExist(nonexistent)
+        assert not self.instance.checkPixelMask(nonexistent)
+
+        # raises an error if the workspace is not a MaskWorkspace
+        notamask = mtd.unique_name(prefix="_mask_check_")
+        CreateSampleWorkspace(
+            OutputWorkspace=notamask,
+            NumBanks=1,
+            BankPixelWidth=1,
+        )
+        assert mtd.doesExist(notamask)
+        assert not isinstance(mtd[notamask], MaskWorkspace)
+        assert not self.instance.checkPixelMask(notamask)
+
+        # return False if nothing is masked
+        emptymask = mtd.unique_name(prefix="_mask_check_")
+        ExtractMask(InputWorkspace=notamask, OutputWorkspace=emptymask)
+        assert isinstance(mtd[emptymask], MaskWorkspace)
+        assert mtd[emptymask].getNumberMasked() == 0
+        assert not self.instance.checkPixelMask(emptymask)
+
+        # return True if something is masked
+        nonemptymask = mtd.unique_name(prefix="_mask_check_")
+        MaskDetectors(Workspace=notamask, WorkspaceIndexList=[0])
+        ExtractMask(InputWorkspace=notamask, OutputWorkspace=nonemptymask)
+        assert isinstance(mtd[nonemptymask], MaskWorkspace)
+        assert mtd[nonemptymask].getNumberMasked() != 0
+        assert self.instance.checkPixelMask(nonemptymask)