- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2
 
Reorganise Dream reduction workflows #185
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
Conversation
…for Powder and Geant4 workflows
        
          
                src/ess/dream/workflows/__init__.py
              
                Outdated
          
        
      | from .generic import DreamGenericWorkflow | ||
| from .powder import ( | ||
| DreamGeant4MonitorHistogramWorkflow, | ||
| DreamGeant4MonitorIntegratedWorkflow, | ||
| DreamGeant4ProtonChargeWorkflow, | ||
| DreamGeant4Workflow, | ||
| DreamPowderWorkflow, | ||
| ) | ||
| 
               | 
          ||
| __all__ = [ | ||
| 'DreamGeant4MonitorHistogramWorkflow', | ||
| 'DreamGeant4MonitorIntegratedWorkflow', | ||
| 'DreamGeant4ProtonChargeWorkflow', | ||
| 'DreamGeant4Workflow', | ||
| 'DreamGenericWorkflow', | ||
| 'DreamPowderWorkflow', | ||
| ] | 
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.
Not sure why we need this subfolder. Confusing why some workflows are in dream, others only in dream.workflows. Why not have everything in dream? We won't have 1000 workflows so I think that should be fine?
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.
I thought I moved all the workflows into the workflows folder?
In any case, I don't mind putting everything back into the workflow.py file one level above, if you think that's better.
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.
My comment was about which you have in the respective __init__.py on the various levels. Also, we should avoid too much user-visible nesting, since it is just complicated and confusing.
        
          
                src/ess/dream/workflows/generic.py
              
                Outdated
          
        
      | ) | ||
| 
               | 
          ||
| 
               | 
          ||
| def DreamGenericWorkflow() -> sciline.Pipeline: | 
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.
DreamWorkflow?
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.
I thought having Generic in there made it a little more obvious that this should be used as a base for other workflows, but I guess in principle you can also use it on its own.
So I don't have a super strong opinion.
        
          
                src/ess/dream/workflows/generic.py
              
                Outdated
          
        
      | Dream generic workflow with default parameters. | ||
| The workflow is based on the GenericTofWorkflow. | 
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.
Maybe explain what it can do, i.e., load and compute Tof?
        
          
                tests/dream/io/geant4_test.py
              
                Outdated
          
        
      | def test_geant4_in_workflow(file_path, file): | ||
| wf = DreamGenericWorkflow() | ||
| for provider in geant4_providers: | ||
| wf.insert(provider) | ||
| wf[Filename[SampleRun]] = file_path | ||
| wf[NeXusDetectorName] = NeXusDetectorName("mantle") | ||
| wf[NeXusComponent[snx.NXsample, SampleRun]] = sc.DataGroup( | ||
| position=sc.vector([0.0, 0.0, 0.0], unit="mm") | ||
| ) | ||
| pipeline[NeXusComponent[snx.NXsource, SampleRun]] = sc.DataGroup( | ||
| wf[NeXusComponent[snx.NXsource, SampleRun]] = sc.DataGroup( | ||
| position=sc.vector([-3.478, 0.0, -76550], unit="mm") | ||
| ) | 
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.
Not sure why this is not using the Geant4 workflows, where these things are defined already?
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.
Will change, thanks
After introducing the
GenericTofWorkflow, the olddream.io.nexus.LoadNeXusWorkflowwhich reshaped detector banks was no longer used.This PR removed that old nexus workflow, and re-implements bank reshaping in the new workflows.
We now have a
DreamGenericWorkflowwhich is basically theGenericTofWorkflowwith detector bank reshaping.Then, the
DreamPowderWorkflowand theDreamGeant4Workflowbuild on top of this workflow.I did not have to touch the docs, so hopefully this means the API didn't change?