Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Create Artificial Norm Algo within Reduction Workflow #463

Open
wants to merge 33 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
aa8a11c
Story deliverables.
darshdinger Oct 2, 2024
f0b05e5
Few updates
darshdinger Oct 2, 2024
7bd709d
Many tests.
darshdinger Oct 3, 2024
1b22e6f
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 3, 2024
95af271
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 8, 2024
87f8b05
Delete extra test.
darshdinger Oct 8, 2024
ffdcc70
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 8, 2024
b10fa8f
Many updates along with UI stuff.
darshdinger Oct 9, 2024
1a163fd
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 9, 2024
415a9a8
Fixes.
darshdinger Oct 9, 2024
43fd73a
Fix
darshdinger Oct 9, 2024
204b7ce
More fixes.
darshdinger Oct 9, 2024
9e14929
Many more udpates to workflow.
darshdinger Oct 11, 2024
2b25773
More updates. Now for some verfication and fixing some tests.
darshdinger Oct 14, 2024
8c27c74
Updates to tests, checking code cov now
darshdinger Oct 14, 2024
34bc6cf
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 14, 2024
7ee1dd9
Fix tests and satisfy code cov
darshdinger Oct 14, 2024
6fe5a28
More tests...
darshdinger Oct 14, 2024
4dda2b6
Delete tests/resources/outputs/APIServicePaths.json.new
darshdinger Oct 14, 2024
19979ee
more code cov...
darshdinger Oct 14, 2024
a731d37
Possibly now?
darshdinger Oct 14, 2024
351b9f9
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 14, 2024
fab6f90
Plz, hope this is the last one for code cov
darshdinger Oct 14, 2024
c9f5ad4
Now?
darshdinger Oct 14, 2024
7954024
?
darshdinger Oct 14, 2024
40c0587
??
darshdinger Oct 14, 2024
f3da7eb
update to test_Decorators
darshdinger Oct 14, 2024
3b8f5e1
Plzz.
darshdinger Oct 14, 2024
77a5454
Added another test..
darshdinger Oct 14, 2024
0158a87
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 14, 2024
ebb08db
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 15, 2024
bd2e947
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 22, 2024
3942382
Merge branch 'next' into ewm7123-implement-artificial-norm-in-Reduction
darshdinger Oct 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from pydantic import BaseModel

from snapred.meta.mantid.WorkspaceNameGenerator import WorkspaceName


class CreateArtificialNormalizationRequest(BaseModel):
runNumber: str
useLiteMode: bool
peakWindowClippingSize: int
smoothingParameter: float
decreaseParameter: bool = True
lss: bool = True
diffractionWorkspace: WorkspaceName

class Config:
arbitrary_types_allowed = True # Allow arbitrary types like WorkspaceName
extra = "forbid" # Forbid extra fields
1 change: 1 addition & 0 deletions src/snapred/backend/dao/request/ReductionRequest.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class ReductionRequest(BaseModel):
versions: Versions = Versions(None, None)

pixelMasks: List[WorkspaceName] = []
artificialNormalization: Optional[WorkspaceName] = None

# TODO: Move to SNAPRequest
continueFlags: Optional[ContinueWarning.Type] = ContinueWarning.Type.UNSET
Expand Down
13 changes: 13 additions & 0 deletions src/snapred/backend/dao/response/ArtificialNormResponse.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from pydantic import BaseModel, ConfigDict

from snapred.meta.mantid.WorkspaceNameGenerator import WorkspaceName


class ArtificialNormResponse(BaseModel):
diffractionWorkspace: WorkspaceName

model_config = ConfigDict(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably pick one or the other format for specifying model configs.
I like the class based one here but I think we use the above more often in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Updated

extra="forbid",
# required in order to use 'WorkspaceName'
arbitrary_types_allowed=True,
)
13 changes: 9 additions & 4 deletions src/snapred/backend/error/StateValidationException.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@
class StateValidationException(Exception):
"Raised when an Instrument State is invalid"

def __init__(self, exception: Exception):
exceptionStr = str(exception)
tb = exception.__traceback__
def __init__(self, exception):
# Handle both string and Exception types for 'exception'
if isinstance(exception, Exception):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should be expanding exceptions type to be both Exception and str, I dont think I see the usecase in this pr? I just see this used in a test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been reverted

exceptionStr = str(exception)
tb = exception.__traceback__
else:
exceptionStr = str(exception)
tb = None

if tb is not None:
tb_info = traceback.extract_tb(tb)
Expand All @@ -24,7 +29,7 @@ def __init__(self, exception: Exception):
else:
filePath, lineNumber, functionName = None, lineNumber, functionName
else:
filePath, lineNumber, functionName = None, None, None
filePath, lineNumber, functionName = None, None, None # noqa: F841

doesFileExist, hasWritePermission = self._checkFileAndPermissions(filePath)

Expand Down
5 changes: 5 additions & 0 deletions src/snapred/backend/recipe/GenericRecipe.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from snapred.backend.log.logger import snapredLogger
from snapred.backend.recipe.algorithm.BufferMissingColumnsAlgo import BufferMissingColumnsAlgo
from snapred.backend.recipe.algorithm.CalibrationMetricExtractionAlgorithm import CalibrationMetricExtractionAlgorithm
from snapred.backend.recipe.algorithm.CreateArtificialNormalizationAlgo import CreateArtificialNormalizationAlgo
from snapred.backend.recipe.algorithm.DetectorPeakPredictor import DetectorPeakPredictor
from snapred.backend.recipe.algorithm.FitMultiplePeaksAlgorithm import FitMultiplePeaksAlgorithm
from snapred.backend.recipe.algorithm.FocusSpectraAlgorithm import FocusSpectraAlgorithm
Expand Down Expand Up @@ -104,3 +105,7 @@ class ConvertTableToMatrixWorkspaceRecipe(GenericRecipe[ConvertTableToMatrixWork

class BufferMissingColumnsRecipe(GenericRecipe[BufferMissingColumnsAlgo]):
pass


class ArtificialNormalizationRecipe(GenericRecipe[CreateArtificialNormalizationAlgo]):
pass
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def PyInit(self):
"OutputWorkspace",
"",
Direction.Output,
PropertyMode.Mandatory,
PropertyMode.Optional,
validator=WorkspaceUnitValidator("dSpacing"),
),
doc="Workspace that contains artificial normalization.",
Expand All @@ -58,7 +58,9 @@ def chopInredients(self, ingredientsStr: str):

def unbagGroceries(self):
self.inputWorkspaceName = self.getPropertyValue("InputWorkspace")
self.outputWorkspaceName = self.getPropertyValue("OutputWorkspace")
self.outputWorkspaceName = (
self.getPropertyValue("OutputWorkspace") or self.inputWorkspaceName + "_artificial_norm"
)

def peakClip(self, data, winSize: int, decrese: bool, LLS: bool, smoothing: float):
# Clipping peaks from the data with optional smoothing and transformations
Expand Down Expand Up @@ -135,6 +137,7 @@ def PyExec(self):
smoothing=self.smoothingParameter,
)
self.outputWorkspace.setY(i, clippedData)
self.outputWorkspace.setDistribution(True)

# Set the output workspace property
self.setProperty("OutputWorkspace", self.outputWorkspaceName)
Expand Down
187 changes: 135 additions & 52 deletions src/snapred/backend/service/ReductionService.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@
from pathlib import Path
from typing import Any, Dict, List

from snapred.backend.dao.ingredients import GroceryListItem, ReductionIngredients
from snapred.backend.dao.ingredients import (
ArtificialNormalizationIngredients,
GroceryListItem,
ReductionIngredients,
)
from snapred.backend.dao.reduction.ReductionRecord import ReductionRecord
from snapred.backend.dao.request import (
CreateArtificialNormalizationRequest,
FarmFreshIngredients,
ReductionExportRequest,
ReductionRequest,
)
from snapred.backend.dao.request.ReductionRequest import Versions
from snapred.backend.dao.response.ArtificialNormResponse import ArtificialNormResponse
from snapred.backend.dao.response.ReductionResponse import ReductionResponse
from snapred.backend.dao.SNAPRequest import SNAPRequest
from snapred.backend.data.DataExportService import DataExportService
Expand All @@ -20,6 +26,9 @@
from snapred.backend.error.StateValidationException import StateValidationException
from snapred.backend.log.logger import snapredLogger
from snapred.backend.recipe.algorithm.MantidSnapper import MantidSnapper
from snapred.backend.recipe.GenericRecipe import (
ArtificialNormalizationRecipe,
)
from snapred.backend.recipe.ReductionRecipe import ReductionRecipe
from snapred.backend.service.Service import Service
from snapred.backend.service.SousChef import SousChef
Expand Down Expand Up @@ -72,6 +81,7 @@ def __init__(self):
self.registerPath("checkWritePermissions", self.checkWritePermissions)
self.registerPath("getSavePath", self.getSavePath)
self.registerPath("getStateIds", self.getStateIds)
self.registerPath("artificialNormalization", self.artificialNormalization)
return

@staticmethod
Expand All @@ -85,42 +95,67 @@ def validateReduction(self, request: ReductionRequest):
:param request: a reduction request
:type request: ReductionRequest
"""
continueFlags = ContinueWarning.Type.UNSET
# check if a normalization is present
if not self.dataFactoryService.normalizationExists(request.runNumber, request.useLiteMode):
continueFlags |= ContinueWarning.Type.MISSING_NORMALIZATION
# check if a diffraction calibration is present
if not self.dataFactoryService.calibrationExists(request.runNumber, request.useLiteMode):
continueFlags |= ContinueWarning.Type.MISSING_DIFFRACTION_CALIBRATION

# remove any continue flags that are present in the request by xor-ing with the flags
if request.continueFlags:
continueFlags = continueFlags ^ (request.continueFlags & continueFlags)

if continueFlags:
raise ContinueWarning(
"Reduction is missing calibration data, continue in uncalibrated mode?", continueFlags
)
if request.artificialNormalization is not None:
continueFlags = ContinueWarning.Type.UNSET

# check that the user has write permissions to the save directory
if not self.checkWritePermissions(request.runNumber):
continueFlags |= ContinueWarning.Type.NO_WRITE_PERMISSIONS

# remove any continue flags that are present in the request by xor-ing with the flags
if request.continueFlags:
continueFlags = continueFlags ^ (request.continueFlags & continueFlags)

if continueFlags:
raise ContinueWarning(
f"<p>Remeber, you don't have permissions to write to "
f"<br><b>{self.getSavePath(request.runNumber)}</b>,<br>"
+ "but you can still save using the workbench tools.</p>"
+ "<p>Would you like to continue anyway?</p>",
continueFlags,
)
else:
continueFlags = ContinueWarning.Type.UNSET
warningMessages = []

# check if a normalization is present
if not self.dataFactoryService.normalizationExists(request.runNumber, request.useLiteMode):
continueFlags |= ContinueWarning.Type.MISSING_NORMALIZATION
warningMessages.append("Normalization is missing, continuing with artificial normalization step.")
# check if a diffraction calibration is present
if not self.dataFactoryService.calibrationExists(request.runNumber, request.useLiteMode):
continueFlags |= ContinueWarning.Type.MISSING_DIFFRACTION_CALIBRATION
warningMessages.append("Diffraction calibration is missing, continuing with uncalibrated mode.")

# remove any continue flags that are present in the request by xor-ing with the flags
if request.continueFlags:
continueFlags = continueFlags ^ (request.continueFlags & continueFlags)

if continueFlags:
detailedMessage = "\n".join(warningMessages)
raise ContinueWarning(
f"The reduction cannot proceed due to missing data:\n{detailedMessage}\n", continueFlags
)

# ... ensure separate continue warnings ...
continueFlags = ContinueWarning.Type.UNSET
# ... ensure separate continue warnings ...
continueFlags = ContinueWarning.Type.UNSET

# check that the user has write permissions to the save directory
if not self.checkWritePermissions(request.runNumber):
continueFlags |= ContinueWarning.Type.NO_WRITE_PERMISSIONS
# check that the user has write permissions to the save directory
if not self.checkWritePermissions(request.runNumber):
continueFlags |= ContinueWarning.Type.NO_WRITE_PERMISSIONS

# remove any continue flags that are present in the request by xor-ing with the flags
if request.continueFlags:
continueFlags = continueFlags ^ (request.continueFlags & continueFlags)
# remove any continue flags that are present in the request by xor-ing with the flags
if request.continueFlags:
continueFlags = continueFlags ^ (request.continueFlags & continueFlags)

if continueFlags:
raise ContinueWarning(
f"<p>It looks like you don't have permissions to write to "
f"<br><b>{self.getSavePath(request.runNumber)}</b>,<br>"
+ "but you can still save using the workbench tools.</p>"
+ "<p>Would you like to continue anyway?</p>",
continueFlags,
)
if continueFlags:
raise ContinueWarning(
f"<p>It looks like you don't have permissions to write to "
f"<br><b>{self.getSavePath(request.runNumber)}</b>,<br>"
+ "but you can still save using the workbench tools.</p>"
+ "<p>Would you like to continue anyway?</p>",
continueFlags,
)
Comment on lines +98 to +158
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and this are duplicate code. Please break this method out into smaller sub methods to reduce its complexity and the amount of nesting.
i.e. this should be its own method, and this should be another method, and those two should be substituted into the above method like so(I reorged it so we didnt need to check write perms twice):

if request.artificialNormalization is None:
    self._validateCalibrationAndNormalizationExists(request)

self._validateWritePermissions(request)


@FromString
def reduction(self, request: ReductionRequest):
Expand All @@ -137,6 +172,9 @@ def reduction(self, request: ReductionRequest):
ingredients = self.prepReductionIngredients(request)

groceries = self.fetchReductionGroceries(request)
if isinstance(groceries, ArtificialNormResponse):
return groceries
Comment on lines 174 to +176
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again we shouldnt be expanding type like this. I'll probably have another comment when I get to prepReductionIngredients.


# attach the list of grouping workspaces to the grocery dictionary
groceries["groupingWorkspaces"] = groupingResults["groupingWorkspaces"]

Expand Down Expand Up @@ -308,8 +346,9 @@ def fetchReductionGroceries(self, request: ReductionRequest) -> Dict[str, Any]:
:rtype: Dict[str, Any]
"""
# Fetch pixel masks
residentMasks = {}
combinedMask = None
residentMasks = {}
# Check for existing pixel masks
if request.pixelMasks:
for mask in request.pixelMasks:
match mask.tokens("workspaceType"):
Expand Down Expand Up @@ -341,30 +380,61 @@ def fetchReductionGroceries(self, request: ReductionRequest) -> Dict[str, Any]:

# As an interim solution: set the request "versions" field to the latest calibration and normalization versions.
# TODO: set these when the request is initially generated.
calVersion = None
normVersion = None
calVersion = self.dataFactoryService.getThisOrLatestCalibrationVersion(request.runNumber, request.useLiteMode)
self.groceryClerk.name("diffcalWorkspace").diffcal_table(request.runNumber, calVersion).useLiteMode(
request.useLiteMode
).add()

if ContinueWarning.Type.MISSING_NORMALIZATION not in request.continueFlags:
normVersion = self.dataFactoryService.getThisOrLatestNormalizationVersion(
if request.artificialNormalization is not None:
calVersion = None
normVersion = 0
calVersion = self.dataFactoryService.getThisOrLatestCalibrationVersion(
request.runNumber, request.useLiteMode
)
self.groceryClerk.name("normalizationWorkspace").normalization(request.runNumber, normVersion).useLiteMode(
self.groceryClerk.name("diffcalWorkspace").diffcal_table(request.runNumber, calVersion).useLiteMode(
request.useLiteMode
).add()
return self.groceryService.fetchGroceryDict(
groceryDict=self.groceryClerk.buildDict(),
normalizationWorkspace=request.artificialNormalization,
**({"maskWorkspace": combinedMask} if combinedMask else {}),
)

request.versions = Versions(
calVersion,
normVersion,
)
else:
calVersion = None
normVersion = None
calVersion = self.dataFactoryService.getThisOrLatestCalibrationVersion(
request.runNumber, request.useLiteMode
)
self.groceryClerk.name("diffcalWorkspace").diffcal_table(request.runNumber, calVersion).useLiteMode(
request.useLiteMode
).add()
Comment on lines +399 to +406
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very similar code to the above, I dont think normVersion is used in either of the highlighted snippets, so we can extract this duplicate code out and above the if/else statement.


return self.groceryService.fetchGroceryDict(
groceryDict=self.groceryClerk.buildDict(),
**({"maskWorkspace": combinedMask} if combinedMask else {}),
)
if ContinueWarning.Type.MISSING_NORMALIZATION not in request.continueFlags:
normVersion = self.dataFactoryService.getThisOrLatestNormalizationVersion(
request.runNumber, request.useLiteMode
)
self.groceryClerk.name("normalizationWorkspace").normalization(
request.runNumber, normVersion
).useLiteMode(request.useLiteMode).add()
elif calVersion and normVersion is None:
groceryList = (
self.groceryClerk.name("diffractionWorkspace")
.diffcal_output(request.runNumber, calVersion)
.useLiteMode(request.useLiteMode)
.unit(wng.Units.DSP)
.group("column")
.buildDict()
)

groceries = self.groceryService.fetchGroceryDict(groceryList)
diffractionWorkspace = groceries.get("diffractionWorkspace")
return ArtificialNormResponse(diffractionWorkspace=diffractionWorkspace)

request.versions = Versions(
calVersion,
normVersion,
)

return self.groceryService.fetchGroceryDict(
groceryDict=self.groceryClerk.buildDict(),
**({"maskWorkspace": combinedMask} if combinedMask else {}),
)

def saveReduction(self, request: ReductionExportRequest):
self.dataExportService.exportReductionRecord(request.record)
Expand Down Expand Up @@ -424,3 +494,16 @@ def _groupByVanadiumVersion(self, requests: List[SNAPRequest]):
def getCompatibleMasks(self, request: ReductionRequest) -> List[WorkspaceName]:
runNumber, useLiteMode = request.runNumber, request.useLiteMode
return self.dataFactoryService.getCompatibleReductionMasks(runNumber, useLiteMode)

def artificialNormalization(self, request: CreateArtificialNormalizationRequest):
ingredients = ArtificialNormalizationIngredients(
peakWindowClippingSize=request.peakWindowClippingSize,
smoothingParameter=request.smoothingParameter,
decreaseParameter=request.decreaseParameter,
lss=request.lss,
)
artificialNormWorkspace = ArtificialNormalizationRecipe().executeRecipe(
InputWorkspace=request.diffractionWorkspace,
Ingredients=ingredients,
)
return artificialNormWorkspace
4 changes: 4 additions & 0 deletions src/snapred/ui/view/BackendRequestView.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from snapred.ui.widget.LabeledField import LabeledField
from snapred.ui.widget.MultiSelectDropDown import MultiSelectDropDown
from snapred.ui.widget.SampleDropDown import SampleDropDown
from snapred.ui.widget.TrueFalseDropDown import TrueFalseDropDown


class BackendRequestView(QWidget):
Expand Down Expand Up @@ -33,6 +34,9 @@ def _labeledCheckBox(self, label):
def _sampleDropDown(self, label, items=[]):
return SampleDropDown(label, items, self)

def _trueFalseDropDown(self, label):
return TrueFalseDropDown(label, self)

def _multiSelectDropDown(self, label, items=[]):
return MultiSelectDropDown(label, items, self)

Expand Down
Loading