Skip to content

Conversation

@nvaytet
Copy link
Member

@nvaytet nvaytet commented Jun 21, 2024

Adds PowgenWorkflow() and DreamGeant4Workflow() helpers.

I also moved the ess/powder/external folder to ess/snspowder to be more like ess/isissans in esssans.

Copy link
Member

@SimonHeybrock SimonHeybrock left a comment

Choose a reason for hiding this comment

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

Small comments:

Comment on lines 37 to 58
def default_parameters() -> dict:
# QUantities not available in the simulated data
sample = sc.DataGroup(position=sc.vector([0.0, 0.0, 0.0], unit="mm"))
source = sc.DataGroup(position=sc.vector([-3.478, 0.0, -76550], unit="mm"))
charge = sc.scalar(1.0, unit="µAh")
return {
RawSample[SampleRun]: sample,
RawSample[VanadiumRun]: sample,
RawSource[SampleRun]: source,
RawSource[VanadiumRun]: source,
AccumulatedProtonCharge[SampleRun]: charge,
AccumulatedProtonCharge[VanadiumRun]: charge,
}


def DreamGeant4Workflow() -> sciline.Pipeline:
"""
Workflow with default parameters for the Powgen SNS instrument.
"""
return sciline.Pipeline(
providers=powder_providers + geant4_providers, params=default_parameters()
)
Copy link
Member

Choose a reason for hiding this comment

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

  • Does this belong into the __init__.py?
  • Docstring refers to Powgen.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Does this belong into the __init__.py?

hmm in essans, we have a separate workflow file for loki, in essreflectometry, for Amor, we have the workflow in the __init__.py. We should be consistent and just have a separate workflow file?

  • Docstring refers to Powgen.

Bad copy/paste ;-)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think consistency is the main argument, I thought the __init__.py should generally be kept minimal?

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the workflows be listed in the api-reference?

@SimonHeybrock SimonHeybrock self-assigned this Jun 24, 2024
"# Mask in time-of-flight to crop to valid range\n",
"workflow[TofMask] = lambda x: (x < sc.scalar(0.0, unit=\"ns\")) | (x > sc.scalar(86e6, unit=\"ns\"))\n",
"workflow[TwoThetaMask] = None\n",
"workflow[WavelengthMask] = None\n",
Copy link
Member

Choose a reason for hiding this comment

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

Can those be None by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

They could, but I thought this would make it obvious what the syntax is if you want to add other masks. I guess this goes back to the discussion on whether these notebooks should only show what is needed, and we need another page that describes everything that can be done in the workflow.

@nvaytet nvaytet merged commit 8b7efc6 into main Jun 25, 2024
@nvaytet nvaytet deleted the workflow-helper branch June 25, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants