Use generic tof workflow

[nvaytet]

Needs scipp/essreduce#233

[jl-wynen]

Looks good, just some minor comments.

[jl-wynen]

Is this the 2d table?

[nvaytet]

Yes, with additional coords (I think I updated the notebook that generates it?)

[jl-wynen]

Why?

[nvaytet]

Left over from local development.
It's an issue for me because of the way I locally 'installed' our packages (using symbolic links instead of a pip install).
I'll change it back.

[nvaytet]

The issue with the docs build is apparently when I insert this provider

Without it, no error in the docs.
I don't understand what is going on. The get_possible_outputs should be sorting a tuple with strings, there shouldn't be an issue.
If I print the content of get_possible_outputs, before it tries to sort it, I get a long list of types, but there is no _GenericAlias anywhere in that list... 🤔

[SimonHeybrock]

The issue with the docs build is apparently when I insert this provider

Without it, no error in the docs. I don't understand what is going on. The get_possible_outputs should be sorting a tuple with strings, there shouldn't be an issue. If I print the content of get_possible_outputs, before it tries to sort it, I get a long list of types, but there is no _GenericAlias anywhere in that list... 🤔

No generic type?

[SimonHeybrock]

Is this still used? If so, why is there no such provider in essreduce?

[SimonHeybrock]

Is this not generic across instruments? Why not define in essreduce?

[nvaytet]

Yes, let's do that

[nvaytet]

So you can either load a lookup table from file, or build it from a simulation.

I'm assuming that the default should be that you want to load the table from a file (this is what most people will want to do).
What would be a convenient way to switch between this mode, and building from simulation?

At the moment, i'm having to know which provider to import and then insert in the workflow, as done in the tests here: https://github.com/scipp/essdiffraction/pull/160/files#diff-397d502aff143b028a421a4e43ded449baae3d8b2ae6711bee74734d7aa1c9e5R146

Is this ok for now? It's not many lines of code, but knowing the import path is annoying. Should I add that particular provider to the __init__ of the time_of_flight submodule to make it more accessible?

[SimonHeybrock]

When computing the LUT, don't you actually need a whole bunch of extra bits in the pipeline, such as loading choppers?

[nvaytet]

Hmm yes. It seems that in the test here I've stopped the workflow at the SimulationResults, which is computed once, without reading choppers from the files.

However, in the TBL workflow in essimaging, I'm using the choppers from the file in a provider to make the SimulationResults: https://github.com/scipp/essimaging/blob/main/src/ess/tbl/conversion.py#L66
Looking at that provider, I'm thinking I should probably also read the source_position from the nexus file.

So we would need to insert at least these two providers. Should we make 2 different workflows? One that loads lookup table from file, and one that builds it on-the-fly?

[SimonHeybrock]

You can also consider adding a keyword argument so this can be selecting on workflow creation.

[nvaytet]

That would probably be convenient. However, does this mean I now have 6 cases instead of the 3 here?

@register_workflow
def DreamGeant4MonitorHistogramWorkflow() -> sciline.Pipeline:
    """
    Workflow with default parameters for the Dream Geant4 simulation, using a
    histogrammed monitor for the normalization.
    """
    return DreamGeant4Workflow(run_norm=RunNormalization.monitor_histogram)


@register_workflow
def DreamGeant4MonitorIntegratedWorkflow() -> sciline.Pipeline:
    """
    Workflow with default parameters for the Dream Geant4 simulation, using
    integrated counts of the monitor for the normalization.
    """
    return DreamGeant4Workflow(run_norm=RunNormalization.monitor_integrated)


@register_workflow
def DreamGeant4ProtonChargeWorkflow() -> sciline.Pipeline:
    """
    Workflow with default parameters for the Dream Geant4 simulation, using
    proton charge for the normalization.
    """
    return DreamGeant4Workflow(run_norm=RunNormalization.proton_charge)

Won't this grow exponentially?
I guess this is mostly for the widgets, it doesn't really affect the rest if I add an argument to the underlying DreamGeant4Workflow?

[SimonHeybrock]

You are right that some things are a bit too inflexible right now. Maybe we need to support workflow (creation) parameters in the widgets?

[nvaytet]

Looks good.
Again, can't approve.
Note that dependencies will need updating again after we release essreduce==25.05.1.

[SimonHeybrock]

Updated essreduce, should be good to go if builds pass.