un-reverso the removeBackground flag (#473)

* un-reverso the removeBackground flag

* rebin the dpsaced data to avoid errors at low-d edge

* make it a toggle

* fixup! make it a toggle

* disable the toggle until remove event background fixed

---------

Co-authored-by: Boston S R <4rx@analysis-node19.sns.gov>

src/snapred/backend/dao/ingredients/DiffractionCalibrationIngredients.py

@@ -1,6 +1,6 @@
 from typing import List
 
-from pydantic import BaseModel, ConfigDict
+from pydantic import BaseModel
 
 from snapred.backend.dao.GroupPeakList import GroupPeakList
 from snapred.backend.dao.RunConfig import RunConfig
@@ -9,7 +9,7 @@
 from snapred.meta.mantid.AllowedPeakTypes import SymmetricPeakEnum
 
 
-class DiffractionCalibrationIngredients(BaseModel):
+class DiffractionCalibrationIngredients(BaseModel, extra="forbid"):
     """
 
     The DiffractionCalibrationIngredients class encapsulates all the necessary components for
@@ -29,8 +29,4 @@ class DiffractionCalibrationIngredients(BaseModel):
     maxOffset: float = Config["calibration.diffraction.maximumOffset"]
     maxChiSq: float = Config["constants.GroupDiffractionCalibration.MaxChiSq"]
     skipPixelCalibration: bool = False
-    # NOTE: removeBackground == True means that the background IS NOT removed
-    # NOTE: removeBackgroud == False means that the background IS removed
-    removeBackground: bool = True
-
-    model_config = ConfigDict(extra="forbid")
+    removeBackground: bool = False

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

@@ -1,7 +1,7 @@
 # TODO this can probably be relaced in the code with FarmFreshIngredients
 from typing import Any, Optional
 
-from pydantic import BaseModel, ConfigDict, field_validator
+from pydantic import BaseModel, field_validator
 
 from snapred.backend.dao.Limit import Pair
 from snapred.backend.dao.state.FocusGroup import FocusGroup
@@ -36,7 +36,7 @@ class DiffractionCalibrationRequest(BaseModel, extra="forbid"):
     fwhmMultipliers: Pair[float] = Pair.model_validate(Config["calibration.parameters.default.FWHMMultiplier"])
     maxChiSq: float = Config["constants.GroupDiffractionCalibration.MaxChiSq"]
     skipPixelCalibration: bool = False
-    removeBackground: Optional[bool]
+    removeBackground: bool = False
 
     continueFlags: Optional[ContinueWarning.Type] = ContinueWarning.Type.UNSET
 
@@ -49,5 +49,3 @@ def validate_fwhmMultipliers(cls, v: Any) -> Pair[float]:
             # Coerce Generic[T]-derived type
             v = Pair[float](**v.dict())
         return v
-
-    model_config = ConfigDict(extra="forbid")

src/snapred/backend/recipe/PixelDiffCalRecipe.py

@@ -84,26 +84,16 @@ def unbagGroceries(self, groceries: Dict[str, WorkspaceName]) -> None:  # noqa A
         self.isEventWs = self.mantidSnapper.mtd[self.wsTOF].id() == "EventWorkspace"
         # the input data converted to d-spacing
         self.wsDSP = wng.diffCalInputDSP().runNumber(self.runNumber).build()
-        self.mantidSnapper.ConvertUnits(
-            "Convert to d-spacing to diffraction focus",
-            InputWorkspace=self.wsTOF,
-            OutPutWorkspace=self.wsDSP,
-            Target="dSpacing",
-        )
+        self.convertUnitsAndRebin(self.wsTOF, self.wsDSP)
         self.mantidSnapper.MakeDirtyDish(
             "Creating copy of initial d-spacing data",
             InputWorkspace=self.wsDSP,
             OutputWorkspace=self.wsDSP + "_startOfPixelDiffCal",
         )
 
-        if not self.removeBackground:
+        if self.removeBackground:
             self.stripBackground(self.detectorPeaks, self.wsTOF, self.groupingWS)
-            self.mantidSnapper.ConvertUnits(
-                "Convert background-subtracted TOF to d-spacing",
-                InputWorkspace=self.wsTOF,
-                OutPutWorkspace=self.wsDSP,
-                Target="dSpacing",
-            )
+            self.convertUnitsAndRebin(self.wsTOF, self.wsDSP)
             self.mantidSnapper.MakeDirtyDish(
                 "Creating copy of background-subtracted d-spacing",
                 InputWorkspace=self.wsDSP,
@@ -173,7 +163,7 @@ def convertUnitsAndRebin(self, inputWS: str, outputWS: str) -> None:
             InputWorkspace=outputWS,
             OutputWorkspace=outputWS,
             Params=self.dSpaceParams,
-            PreserveEvents=self.removeBackground,
+            PreserveEvents=not self.removeBackground,
             BinningMode="Logarithmic",
         )
 

src/snapred/ui/view/DiffCalRequestView.py

@@ -19,7 +19,7 @@ class DiffCalRequestView(BackendRequestView):
 
     """
 
-    def __init__(self, removeBackgroundToggle, samples=[], groups=[], parent=None):
+    def __init__(self, samples=[], groups=[], parent=None):
         super().__init__(parent=parent)
 
         # input fields
@@ -33,10 +33,10 @@ def __init__(self, removeBackgroundToggle, samples=[], groups=[], parent=None):
         self.groupingFileDropdown = self._sampleDropDown("Grouping File", groups)
         self.peakFunctionDropdown = self._sampleDropDown("Peak Function", [p.value for p in SymmetricPeakEnum])
 
-        if removeBackgroundToggle:
-            # checkbox for removing background
-            self.removeBackgroundCheckBox = self._labeledCheckBox("Remove Background?")
-            self.removeBackgroundCheckBox.setChecked(False)
+        # checkbox for removing background
+        # NOTE not enabled until remove event background is fixed
+        self.removeBackgroundToggle = self._labeledField("RemoveBackground", Toggle(parent=self, state=False))
+        self.removeBackgroundToggle.setEnabled(False)
 
         # set field properties
         self.litemodeToggle.setEnabled(True)
@@ -45,13 +45,12 @@ def __init__(self, removeBackgroundToggle, samples=[], groups=[], parent=None):
         # add all widgets to layout
         self.layout.addWidget(self.runNumberField, 0, 0)
         self.layout.addWidget(self.litemodeToggle, 0, 1)
+        self.layout.addWidget(self.removeBackgroundToggle, 0, 2)
         self.layout.addWidget(self.fieldConvergenceThreshold, 1, 0)
-        self.layout.addWidget(self.fieldNBinsAcrossPeakWidth, 1, 2)
+        self.layout.addWidget(self.fieldNBinsAcrossPeakWidth, 1, 1)
         self.layout.addWidget(self.sampleDropdown, 2, 0)
         self.layout.addWidget(self.groupingFileDropdown, 2, 1)
         self.layout.addWidget(self.peakFunctionDropdown, 2, 2)
-        if removeBackgroundToggle:
-            self.layout.addWidget(self.removeBackgroundCheckBox, 0, 2)
 
     def populateGroupingDropdown(self, groups):
         self.groupingFileDropdown.setItems(groups)
@@ -74,4 +73,4 @@ def getLiteMode(self):
         return self.litemodeToggle.field.getState()
 
     def getRemoveBackground(self):
-        return self.removeBackgroundCheckBox.isChecked()
+        return self.removeBackgroundToggle.field.getState()

src/snapred/ui/workflow/DiffCalWorkflow.py

@@ -68,13 +68,11 @@ def __init__(self, parent=None):
         self.defaultGroupingMap = self.request(path="config/groupingMap", payload="tmfinr").data
         self.groupingMap = self.defaultGroupingMap
         self.focusGroups = self.groupingMap.lite
-        self.removeBackground = True  # NOTE: this will NOT subtract the background, this needs to
-        # be false in order for background subtraction to occur.
+        self.removeBackground = False
 
         self.addResetHook(self._resetSaveView)
 
         self._requestView = DiffCalRequestView(
-            removeBackgroundToggle=Config["cis_mode"],
             samples=self.samplePaths,
             groups=list(self.focusGroups.keys()),
             parent=parent,
@@ -201,8 +199,7 @@ def _specifyRun(self, workflowPresenter):
         self.peakFunction = view.peakFunctionDropdown.currentText()
         self.maxChiSq = self.DEFAULT_MAX_CHI_SQ
 
-        if Config["cis_mode"]:
-            self.removeBackground = not view.removeBackgroundCheckBox.isChecked()
+        self.removeBackground = view.getRemoveBackground()
 
         # Validate that the user has write permissions as early as possible in the workflow.
         permissionsRequest = CalibrationWritePermissionsRequest(

tests/cis_tests/diffcal_pixel_diffraction_background_subtraction_script.py

@@ -22,12 +22,12 @@
 #User input ###########################
 runNumber = "58882"
 groupingScheme = "Column"
-cifPath = "/SNS/SNAP/shared/Calibration/CalibrantSamples/Silicon_NIST_640d.cif"
+cifPath = "/SNS/SNAP/shared/Calibration/CalibrantSamples/cif/Silicon_NIST_640d.cif"
 calibrantSamplePath = "Silicon_NIST_640D_001.json"
 peakThreshold = 0.05
 offsetConvergenceLimit = 0.1
 isLite = True
-removeBackground = False #NOTE: True == don't remove background // False == remove background
+removeBackground = True
 Config._config["cis_mode"] = True
 Config._config["diffraction.smoothingParameter"] = 0.01  #This is the smoothing parameter to be set.
 #######################################
@@ -65,4 +65,21 @@
 pixelRx = PixelRx()
 pixelRx.prep(ingredients, groceries)
 pixelRes = pixelRx.execute()
-print(pixelRx.medianOffsets[-1])
+
+### PREPARE OUTPUTS ################
+DiffractionFocussing(
+    InputWorkspace=f"dsp_0{runNumber}_raw_startOfPixelDiffCal",
+    OutputWorkspace="BEFORE_REMOVAL",
+    GroupingWorkspace=groceries["groupingWorkspace"],
+)
+DiffractionFocussing(
+    InputWorkspace=f"dsp_0{runNumber}_raw_withoutBackground",
+    OutputWorkspace="AFTER_REMOVAL",
+    GroupingWorkspace=groceries["groupingWorkspace"],
+)
+DiffractionFocussing(
+    InputWorkspace="tof_all_lite_copy1_058882",
+    OutputWorkspace="FINAL",
+    GroupingWorkspace=groceries["groupingWorkspace"],
+)
+

tests/integration/test_versions_in_order.py

@@ -368,7 +368,7 @@ def run_diffcal(self):
             "focusGroup": groupingMap["Natural"].model_dump(),
             "calibrantSamplePath": Resource.getPath("inputs/calibrantSamples/Silicon_NIST_640D_001.json"),
             "fwhmMultipliers": {"left": 0.5, "right": 0.5},
-            "removeBackground": True,
+            "removeBackground": False,
         }
         request = SNAPRequest(path="calibration/diffraction", payload=json.dumps(payload))
         response = self.api.executeRequest(request)

tests/unit/backend/dao/test_ForbidExtras.py

@@ -0,0 +1,44 @@
+from unittest.mock import sentinel
+
+import pytest
+from pydantic import ValidationError
+from snapred.backend.dao.ingredients.DiffractionCalibrationIngredients import DiffractionCalibrationIngredients
+from snapred.backend.dao.request.DiffractionCalibrationRequest import DiffractionCalibrationRequest
+
+"""
+This is just a collection of tests verifying that extra inputs to certain DAOs
+are fobidden and generate errors.  Pydantic offers several different ways to
+set this, so this test ensures consistent behavior.
+Please add more DAO tests to this file as you encounter them.
+"""
+
+
+def test_forbid_exta_diffcalingredients():
+    with pytest.raises(ValidationError) as e:
+        DiffractionCalibrationIngredients(
+            runConfig=sentinel.runConfig,
+            pixelGroup=sentinel.pixelGroup,
+            groupedPeakLists=sentinel.peakList,
+            notPartOfThis=True,
+        )
+    # there will be errors corresponding to the mandatory arguments
+    # we only want to make sure the last one complains about extras
+    errors = e.value.errors()
+    errors[-1]["loc"] == ["notPartOfThis"]
+    errors[-1]["msg"] = "Extra inputs are not permitted"
+
+
+def test_forbid_exta_diffcalrequest():
+    with pytest.raises(ValidationError) as e:
+        DiffractionCalibrationRequest(
+            runNumber=sentinel.runNumber,
+            calibrationSamplePath=sentinel.calibrationSamplePath,
+            focusGroup=sentinel.focusGroup,
+            useLiteMode=True,
+            notPartOfThis=True,
+        )
+    # there will be errors corresponding to the mandatory arguments
+    # we only want to make sure the last one complains about extras
+    errors = e.value.errors()
+    errors[-1]["loc"] == ["notPartOfThis"]
+    errors[-1]["msg"] = "Extra inputs are not permitted"

tests/unit/backend/service/test_CalibrationService.py

@@ -722,7 +722,7 @@ def test_diffractionCalibration_calls_with_ingredients(self, SimpleDiffCalReques
             runNumber="123",
             useLiteMode=True,
             calibrantSamplePath="bundt/cake_egg.py",
-            removeBackground=True,
+            removeBackground=False,
             focusGroup=FocusGroup(name="all", definition="path/to/all"),
             continueFlags=ContinueWarning.Type.UNSET,
         )
@@ -937,7 +937,7 @@ def test_validateRequest(self):
             runNumber="123",
             useLiteMode=True,
             calibrantSamplePath="bundt/cake_egg.py",
-            removeBackground=True,
+            removeBackground=False,
             focusGroup=FocusGroup(name="all", definition="path/to/all"),
             continueFlags=ContinueWarning.Type.UNSET,
         )
@@ -956,7 +956,7 @@ def test_validateWritePermissions(self):
             runNumber="123",
             useLiteMode=True,
             calibrantSamplePath="bundt/cake_egg.py",
-            removeBackground=True,
+            removeBackground=False,
             focusGroup=FocusGroup(name="all", definition="path/to/all"),
             continueFlags=ContinueWarning.Type.UNSET,
         )
@@ -977,7 +977,7 @@ def test_validateWritePermissions_no_permissions(self):
             runNumber="123",
             useLiteMode=True,
             calibrantSamplePath="bundt/cake_egg.py",
-            removeBackground=True,
+            removeBackground=False,
             focusGroup=FocusGroup(name="all", definition="path/to/all"),
             continueFlags=ContinueWarning.Type.UNSET,
         )
@@ -1018,7 +1018,7 @@ def test_diffractionCalibration_with_bad_masking(
             runNumber="123",
             useLiteMode=True,
             calibrantSamplePath="bundt/cake_egg.py",
-            removeBackground=True,
+            removeBackground=False,
             focusGroup=FocusGroup(name="all", definition="path/to/all"),
             continueFlags=ContinueWarning.Type.UNSET,
             skipPixelCalibration=False,