-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: next
Are you sure you want to change the base?
Changes from all commits
aa8a11c
f0b05e5
7bd709d
1b22e6f
95af271
87f8b05
ffdcc70
b10fa8f
1a163fd
415a9a8
43fd73a
204b7ce
9e14929
2b25773
8c27c74
34bc6cf
7ee1dd9
6fe5a28
4dda2b6
19979ee
a731d37
351b9f9
fab6f90
c9f5ad4
7954024
40c0587
f3da7eb
3b8f5e1
77a5454
0158a87
ebb08db
bd2e947
3942382
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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( | ||
extra="forbid", | ||
# required in order to use 'WorkspaceName' | ||
arbitrary_types_allowed=True, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is used in the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
|
||
|
||
@FromString | ||
def reduction(self, request: ReductionRequest): | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
# attach the list of grouping workspaces to the grocery dictionary | ||
groceries["groupingWorkspaces"] = groupingResults["groupingWorkspaces"] | ||
|
||
|
@@ -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"): | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is very similar code to the above, I dont think |
||
|
||
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) | ||
|
@@ -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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Updated