From b721a421b9eabe28a556cf3a9941b807eba9f6c9 Mon Sep 17 00:00:00 2001 From: Reece Boston <52183986+rboston628@users.noreply.github.com> Date: Tue, 12 Nov 2024 15:33:14 -0500 Subject: [PATCH] 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> --- .../DiffractionCalibrationIngredients.py | 10 ++--- .../request/DiffractionCalibrationRequest.py | 6 +-- .../backend/recipe/PixelDiffCalRecipe.py | 18 ++------ src/snapred/ui/view/DiffCalRequestView.py | 17 ++++--- src/snapred/ui/workflow/DiffCalWorkflow.py | 7 +-- ...ffraction_background_subtraction_script.py | 23 ++++++++-- tests/integration/test_versions_in_order.py | 2 +- tests/unit/backend/dao/test_ForbidExtras.py | 44 +++++++++++++++++++ .../service/test_CalibrationService.py | 10 ++--- 9 files changed, 89 insertions(+), 48 deletions(-) create mode 100644 tests/unit/backend/dao/test_ForbidExtras.py diff --git a/src/snapred/backend/dao/ingredients/DiffractionCalibrationIngredients.py b/src/snapred/backend/dao/ingredients/DiffractionCalibrationIngredients.py index ed5c24c0b..7948f5077 100644 --- a/src/snapred/backend/dao/ingredients/DiffractionCalibrationIngredients.py +++ b/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 diff --git a/src/snapred/backend/dao/request/DiffractionCalibrationRequest.py b/src/snapred/backend/dao/request/DiffractionCalibrationRequest.py index 5fa81c7bf..f8bff02f2 100644 --- a/src/snapred/backend/dao/request/DiffractionCalibrationRequest.py +++ b/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") diff --git a/src/snapred/backend/recipe/PixelDiffCalRecipe.py b/src/snapred/backend/recipe/PixelDiffCalRecipe.py index 7a7c561fb..5d0d9fd7a 100644 --- a/src/snapred/backend/recipe/PixelDiffCalRecipe.py +++ b/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", ) diff --git a/src/snapred/ui/view/DiffCalRequestView.py b/src/snapred/ui/view/DiffCalRequestView.py index abb20df0b..e5aaa6c5a 100644 --- a/src/snapred/ui/view/DiffCalRequestView.py +++ b/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() diff --git a/src/snapred/ui/workflow/DiffCalWorkflow.py b/src/snapred/ui/workflow/DiffCalWorkflow.py index 03f4e88eb..00e55f8f4 100644 --- a/src/snapred/ui/workflow/DiffCalWorkflow.py +++ b/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( diff --git a/tests/cis_tests/diffcal_pixel_diffraction_background_subtraction_script.py b/tests/cis_tests/diffcal_pixel_diffraction_background_subtraction_script.py index ee7dd5ccd..98ef358f6 100644 --- a/tests/cis_tests/diffcal_pixel_diffraction_background_subtraction_script.py +++ b/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"], +) + diff --git a/tests/integration/test_versions_in_order.py b/tests/integration/test_versions_in_order.py index a593da162..4fdbc4009 100644 --- a/tests/integration/test_versions_in_order.py +++ b/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) diff --git a/tests/unit/backend/dao/test_ForbidExtras.py b/tests/unit/backend/dao/test_ForbidExtras.py new file mode 100644 index 000000000..ddb073f03 --- /dev/null +++ b/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" diff --git a/tests/unit/backend/service/test_CalibrationService.py b/tests/unit/backend/service/test_CalibrationService.py index bac6f2162..1a74c887f 100644 --- a/tests/unit/backend/service/test_CalibrationService.py +++ b/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,