Ewm3645 mask workspace diffcal (#658)

* impl: add mask workspace to diffcal

* squash

fix import path

apparently the auto import logic has changed for the worse

fix fetch diffcal groceries unit test

odd update to try and get github to kick off ci

add test for the combining of masks for diffcal

actually pass the maskworkspace to be used by the backend

update tests to accept maskworkspaces from different states as valid

fix integration tests

added a test to confirm resident masks are filtered if not compatible

change exception type to something not pydantic

up the last couple lines of coverage

update precommit, remove commented out code

fix mock method definition

* wip commit to squash later

* enable masking in diffcal workflow, including fully masked groups

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update pixi lock file

* update tests

* fix maskless flow of calibration workflow

* update pixi lockfile

* add request validation for ingredients and groceries so state can init

* fix some other maskworkspace handling

* fix integration tests

* fix existing unit tests

* refactor diffcal happy path integration tests, add one for user mask

* update pixi lock file

* move combine mask logic to grocery service, make some updates according to comments

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix segfaults in test_reductionservice tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix last failing test, make more updates as per comments

* fix issue with mismatched peaklists at assessment time

* update pixi lockfile

* move back pin on ruamel.yaml, seems to be causing doc issues

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* syntax error

* fix precommit checks

* add workaround for ConjoinWorkspaces mantid defect.  require outputws of combinePixelMasks already exist

* fix up 2 failining unit tests

* move some mocks to patches

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix boundry condition in pixeldiffcal assuming index 0 would always be the initial host for offsets

* add exception if the mask fully masks the instrument

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix tests

* update pixi lock file

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix precommit errors

* correctly label group id's on the ui

* account for the additional masking that pixel calibration introduces

* add missing method to test souschef

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: walshmm

.readthedocs.yaml

@@ -21,7 +21,7 @@ build:
       # Install the package in editable mode using pixi
       - $HOME/.pixi/bin/pixi run --environment docs pip install -e .
       # Install docs dependencies in the RTD Python environment too
-      - pip install erdantic versioningit sphinx_rtd_theme sphinxcontrib-mermaid types-pyyaml h5py numpy matplotlib ruamel.yaml
+      - pip install erdantic versioningit sphinx_rtd_theme sphinxcontrib-mermaid types-pyyaml h5py numpy matplotlib ruamel.yaml==0.18.16
 
 sphinx:
   builder: html

pixi.lock

@@ -37,6 +37,7 @@ class CalibrationAssessmentRequest(BaseModel):
         default_factory=lambda: Pair.model_validate(Config["calibration.parameters.default.FWHMMultiplier"])
     )
     maxChiSq: float = Field(default_factory=lambda: Config["constants.GroupDiffractionCalibration.MaxChiSq"])
+    combinedPixelMask: WorkspaceName | None = None
 
     @field_validator("fwhmMultipliers", mode="before")
     @classmethod

src/snapred/backend/dao/request/CalibrationAssessmentRequest.py

@@ -43,6 +43,7 @@ class DiffractionCalibrationRequest(BaseModel):
     maxChiSq: float = Field(default_factory=lambda: Config["constants.GroupDiffractionCalibration.MaxChiSq"])
     removeBackground: bool = False
     pixelMasks: List[WorkspaceName] = []
+    combinedPixelMask: WorkspaceName | None = None
 
     continueFlags: Optional[ContinueWarning.Type] = ContinueWarning.Type.UNSET
 

src/snapred/backend/dao/request/DiffractionCalibrationRequest.py

@@ -1,8 +1,7 @@
-from typing import Optional
-
-from pydantic import BaseModel
+from pydantic import BaseModel, ConfigDict
 
 from snapred.backend.dao.state.FocusGroup import FocusGroup
+from snapred.meta.mantid.WorkspaceNameGenerator import WorkspaceName
 
 
 class FocusSpectraRequest(BaseModel):
@@ -13,4 +12,7 @@ class FocusSpectraRequest(BaseModel):
 
     inputWorkspace: str
     groupingWorkspace: str
-    outputWorkspace: Optional[str] = None
+    maskWorkspace: WorkspaceName | None = None
+    outputWorkspace: WorkspaceName | None = None
+
+    model_config = ConfigDict(arbitrary_types_allowed=True)

src/snapred/backend/dao/request/FocusSpectraRequest.py

@@ -1515,6 +1515,36 @@ def fetchGroceryList(self, groceryList: Iterable[GroceryListItem]) -> List[Works
 
         return groceries
 
+    def combinePixelMasks(self, outputMaskWsName: WorkspaceName, masks2Combine: List[WorkspaceName]):
+        if not masks2Combine:
+            raise ValueError("Internal Error: Lists of masks to combine is empty")
+
+        if not self.mantidSnapper.mtd.doesExist(outputMaskWsName):
+            raise ValueError(
+                (
+                    "Internal Error: outputMaskWs should exist before attempting to combine masks with it. "
+                    "Consider using fetchCompatiblePixelMask to generate it."
+                )
+            )
+
+        for maskWsName in masks2Combine:
+            if maskWsName == outputMaskWsName:
+                continue
+            if not self.mantidSnapper.mtd.doesExist(maskWsName):
+                raise ValueError(
+                    f"Mask {maskWsName} of mask set {masks2Combine} does not exist, cannot combine into pixel mask."
+                )
+            self.mantidSnapper.BinaryOperateMasks(
+                f"combine from pixel mask: '{maskWsName}'...",
+                InputWorkspace1=outputMaskWsName,
+                InputWorkspace2=maskWsName,
+                OperationType="OR",
+                OutputWorkspace=outputMaskWsName,
+            )
+
+        self.mantidSnapper.executeQueue()
+        return outputMaskWsName
+
     def fetchGroceryDict(self, groceryDict: Dict[str, GroceryListItem], **kwargs) -> Dict[str, WorkspaceName]:
         """
         This is the primary method you should use for fetching groceries, in almost all cases.

src/snapred/backend/data/GroceryService.py

@@ -66,7 +66,6 @@ def queueAlgos(self):
         # Step 2: Check for overlapping spectra and manage them
         fitPeaksWorkspace = self.mantidSnapper.mtd[self.fitPeaksDiagnosticWorkspaceName]
         numHistograms = fitPeaksWorkspace.getNumberHistograms()
-        processedSpectra = []
         spectrumDict = {}
 
         for i in range(numHistograms):
@@ -102,9 +101,7 @@ def queueAlgos(self):
 
                 spectrumDict[spectrumId] = singleSpectrumName
 
-            # Append the processed spectrum to the list
-            processedSpectra.append(singleSpectrumName)
-
+        processedSpectra = list(spectrumDict.values())
         # Step 3: Combine all processed spectra into a single workspace
         combinedWorkspace = processedSpectra[0]
         for spectrum in processedSpectra[1:]:

src/snapred/backend/recipe/CalculateDiffCalResidualRecipe.py

@@ -77,7 +77,7 @@ def validateInputs(self, ingredients: Ingredients, groceries: Dict[str, Workspac
             )
 
         diffocWS = self.mantidSnapper.mtd[groceries["groupingWorkspace"]]
-        if groupIDs != diffocWS.getGroupIDs().tolist():
+        if not set(groupIDs).issubset(set(diffocWS.getGroupIDs().tolist())):
             raise RuntimeError(
                 f"Group IDs do not match between peak list and focus WS: '{groupIDs}' vs. '{diffocWS.getGroupIDs()}'"
             )

src/snapred/backend/recipe/GroupDiffCalRecipe.py

@@ -1,15 +1,16 @@
 from typing import Any, Dict, List, Set
 
 import numpy as np
-from pydantic import BaseModel
+from pydantic import BaseModel, ConfigDict
 
 from snapred.backend.dao.ingredients import DiffractionCalibrationIngredients as Ingredients
 from snapred.backend.log.logger import snapredLogger
 from snapred.backend.profiling.ProgressRecorder import ComputationalOrder, WallClockTime
 from snapred.backend.recipe.algorithm.Utensils import Utensils
-from snapred.backend.recipe.Recipe import Recipe, WorkspaceName
+from snapred.backend.recipe.Recipe import Recipe
 from snapred.meta.Config import Config
 from snapred.meta.decorators.classproperty import classproperty
+from snapred.meta.mantid.WorkspaceNameGenerator import WorkspaceName
 from snapred.meta.mantid.WorkspaceNameGenerator import WorkspaceNameGenerator as wng
 
 logger = snapredLogger.getLogger(__name__)
@@ -18,10 +19,12 @@
 class PixelDiffCalServing(BaseModel):
     result: bool
     medianOffsets: List[float]
-    maskWorkspace: str
+    maskWorkspace: WorkspaceName | None = None
     calibrationTable: str
     outputWorkspace: str
 
+    model_config = ConfigDict(arbitrary_types_allowed=True)
+
 
 # Decorating with the `WallClockTime` profiler here somewhat duplicates the objective of the decoration
 #   at `CalibrationService.pixelCalibration`.  However, by default, there will be no logging output from this instance.
@@ -101,11 +104,20 @@ def unbagGroceries(self, groceries: Dict[str, WorkspaceName]) -> None:  # noqa A
         """
         self.wsTOF = groceries["inputWorkspace"]
         self.groupingWS = groceries["groupingWorkspace"]
-        self.maskWS = groceries["maskWorkspace"]
+        self.maskWS = groceries.get("maskWorkspace")
         # the name of the output calibration table
         self.DIFCpixel = groceries["calibrationTable"]
         self.DIFCprev = groceries.get("previousCalibration", "")
         # the input data converted to d-spacing
+
+        if self.maskWS and self.mantidSnapper.mtd.doesExist(self.maskWS):
+            # if user supplied mask exists, apply it
+            # A user mask may not exist, but the maskws name is used to store
+            # the mask generated as part of PixelDiffCalRecipe
+            self.mantidSnapper.MaskDetectors(
+                "applying user generated mask", Workspace=self.wsTOF, MaskedWorkspace=self.maskWS
+            )
+
         self.wsDSP = wng.diffCalInputDSP().runNumber(self.runNumber).build()
         self.convertUnitsAndRebin(self.wsTOF, self.wsDSP)
         self.mantidSnapper.CloneWorkspace(
@@ -215,11 +227,14 @@ def queueAlgos(self):
         wscc: str = f"__{self.runNumber}_tmp_group_CC_{self._counts}"
 
         for i, (groupID, workspaceIndices) in enumerate(self.groupWorkspaceIndices.items()):
+            if groupID not in self.maxDSpaceShifts:
+                # group has been fully masked.
+                continue
             workspaceIndices = list(workspaceIndices)
             refID: int = self.getRefID(workspaceIndices)
 
             self.mantidSnapper.CrossCorrelate(
-                f"Cross-Correlating spectra for {wscc}",
+                f"Cross-Correlating spectra for {wscc}, subgroup {groupID}",
                 InputWorkspace=self.wsDSP,
                 OutputWorkspace=wscc + f"_group{groupID}",
                 ReferenceSpectra=refID,
@@ -230,9 +245,10 @@ def queueAlgos(self):
             )
 
             self.mantidSnapper.GetDetectorOffsets(
-                f"Calculate offset workspace {wsoff}",
+                f"Calculate offset workspace {wsoff}, group {groupID}",
                 InputWorkspace=wscc + f"_group{groupID}",
                 OutputWorkspace=wsoff,
+                # MasKWs is an in/out, this algo will modify it
                 MaskWorkspace=self.maskWS,
                 # Scale the fitting ROI using the expected peak width (including a few possible peaks):
                 XMin=-(self.maxDSpaceShifts[groupID] / self.dBin) * 2.0,
@@ -243,7 +259,7 @@ def queueAlgos(self):
 
             # add in group offsets to total, or begin the sum if none
             # NOTE wsoff has all spectra, with value 0 in those not used in CrossCorrelate
-            if i == 0:
+            if not self.mantidSnapper.mtd.doesExist(self.totalOffsetWS):
                 self.mantidSnapper.CloneWorkspace(
                     f"Starting summation with offset workspace {wsoff}",
                     InputWorkspace=wsoff,
@@ -308,6 +324,12 @@ def execute(self):
                 InstrumentWorkspace=self.wsTOF,
                 CalibrationWorkspace=self.DIFCpixel,
             )
+            self.mantidSnapper.MaskDetectors(
+                "reapplying user generated + get detector offsets mask",
+                Workspace=self.wsTOF,
+                MaskedWorkspace=self.maskWS,
+            )
+
         else:
             logger.warning("Offsets failed to converge monotonically")
 

src/snapred/backend/recipe/PixelDiffCalRecipe.py

@@ -15,8 +15,11 @@
     mtd,
 )
 
+from snapred.backend.log.logger import snapredLogger
 from snapred.meta.mantid.FitPeaksOutput import FIT_PEAK_DIAG_SUFFIX, FitOutputEnum
 
+logger = snapredLogger.getLogger(__name__)
+
 
 class ConjoinDiagnosticWorkspaces(PythonAlgorithm):
     """
@@ -127,12 +130,24 @@ def conjoinMatrixWorkspaces(self, inws, outws, index):
                 InputWorkspace=inws,
                 OutputWorkspace=tmpws,
             )
+        logger.debug(f"{outws} already has spectrum numbers of {mtd[outws].getSpectrumNumbers()}")
+        logger.debug(f"Conjoining {tmpws} with {outws}, adding spectrum numbers of {mtd[tmpws].getSpectrumNumbers()}")
+        specNumbers = list(mtd[outws].getSpectrumNumbers())
+        specNumbers.extend(list(mtd[tmpws].getSpectrumNumbers()))
         ConjoinWorkspaces(
             InputWorkspace1=outws,
             InputWorkspace2=tmpws,
             CheckOverlapping=False,
             CheckMatchingBins=False,  # Not available in 6.11.0.3rc2
         )
+
+        # TODO: Remove when Defect 14460 is resolved.
+        #       There is a defect in ConjoinWorkspaces that incorrectly determines
+        #       if spectrum numbers need to be remapped.
+        for i, specNum in enumerate(specNumbers):
+            mtd[outws].getSpectrum(i).setSpectrumNo(specNum)
+
+        logger.debug(f"resulting spectrum numbers: {mtd[outws].getSpectrumNumbers()}")
         if self.autoDelete and inws in mtd:
             DeleteWorkspace(inws)
         assert outws in mtd