ADR suggestion: Calculation

[henrikjacobsenfys]

General

Analysis does too much. It's a (too complicated) calculation engine, fitter and plotter. It has a convoluted (pun intended) way of handling Convolution objects and ensuring that they are up to date. Extending Analysis to more coordinates in accordance with #177 is near impossible in its current form. The solution is a class to handle calculations, which I will tentatively call Calculation

Calculation

It will take the following inputs:

• coordinates: scipp.Dict: The coordinates for which to calculate the model. Will typically simply be e.g. data['Q',0, 'temperature', 2].coords. energy will be a required coordinate.
• sample_components ComponentCollection | ModelComponent: Typically the relevant ComponentCollection from a SampleModel
• resolution_model ComponentCollection | ModelComponent: Same, but for resolution
• background_model ComponentCollection | ModelComponent: Same, but for background
• model_temperature Parameter: temperature of the model e.g. for detailed balancing
• energy_offset Parameter: energy offset to handle instrument misalignment
• calculation_settings CalculationSettings: see below
• energy np.ndarray | sc.Variable | None: User-defined energy to do the calculation? Maybe? Or perhaps it should be in the coordinates. I'll think a bit about that.

It will have various methods, the ones that come to mind are

• calculate: This will calculate the model and output a np.ndarray (mainly for fitting)
• calculate_components (or something like this): calculate the model and individual components for plotting
  These will be moved from Analysis and updated in accordance with the rest of this ADR suggestion.

CalculationSettings

I may want to collect the settings in a small CalculationSettings class. It will include DetailedBalanceSettings and ConvolutionSettings (#174 ). The CalculationSettings can handle things like whether components of the resolution should be calculated individually or as a sum (#146 ), whether resolution convolution should happen, if individual components should be calculated and so on.

CalculationContext?

I'm considering grouping all the inputs in its own class, tentatuvely named CalculationContext. This will provide all the input required to do the calculation and will be easy to pass around. On the other hand, having Analysis1d create a CalculationContext with the right input and then passing it to Calculation may not be any simpler than having Analysis1d create a Calculation with the right input to begin with. I'll make the first version without it and see how well that goes.

Note on temperature

With the coming addition of temperature as a coordinate, we need to distinguish between model temperature and sample temperature

[rozyczko]

My comments:
Abstraction depth.
The call chain would become: Analysis -> Calculation -> Convolution -> analytical/numerical convolvers.
Understanding "how does my model get evaluated?" now requires tracing through 4 classes instead of 2.

Making Calculation a thin wrapper. The actual computation already lives in Convolution.
If Calculation mainly delegates to Convolution with some parameter plumbing, it becomes a "pass-through" class that adds extra layer without real value. The question is: what does Calculation do that Convolution doesn't already do?

Too many constructor parameters. The proposed Calculation.__init__ takes coordinates, sample_components, resolution_model, background_model, model_temperature, energy_offset, calculation_settings, and energy. The CalculationContext idea "solves" it by adding another class...

Current structure may be good enough. Looking at the actual code: Analysis1d._calculate() and _evaluate_components() are already private methods that delegate to Convolution. The public API (fit(), plot_data_and_model()) is intuitive.
What's really complex is Convolution itself (analytical vs. numerical path selection, detailed balance application).
Extracting Calculation moves the things around but doesn't simplify the actual hard part.

Maybe instead of a full Calculation class, first refactor Convolution itself to have a cleaner API?
Then extract only the fitting-specific logic from Analysis into a Fitter wrapper.
If, after that Analysis still feels too heavy, you could think of extracting the Calculation class.

[henrikjacobsenfys]

Thanks!

The call chain would become: Analysis -> Calculation -> Convolution -> analytical/numerical convolvers.
Understanding "how does my model get evaluated?" now requires tracing through 4 classes instead of 2.

This is a very valid point. My initial reason for wanting this refactor was that calculate() goes through quite a few helper functions before it (sometimes) lands in Convolution. The simplest to me was to take all of the complexity out of Analysis1d and make it simpler. But a good starting point is to just make it simpler and keep it in Analysis1d

what does Calculation do that Convolution doesn't already do?

It handles calculations without convolution as well. Convolution certainly should not have this responsibility too, right?

The difficulty is that convolution is not always carried out. This is either because we're evaluating a BackgroundModel, or because there is no resolution. Both are valid use cases; one is used almost always, the other only sometimes. Someone needs to be in charge of calling the right things, and right now it's a bit of a mess.

Too many constructor parameters. The proposed Calculation.init takes coordinates, sample_components, resolution_model, background_model, model_temperature, energy_offset, calculation_settings, and energy. The CalculationContext idea "solves" it by adding another class...

I've never been convinced CalculationContext was the right solution, just trying to remove complexity, but it's hard...

Maybe instead of a full Calculation class, first refactor Convolution itself to have a cleaner API?

I agree that what goes on inside Convolution with all its numerical/analytical convolvers etc is quite complex, However, I also think that the public API is quite straightforward: Give it the sample and resolution model, the energy on which to calculate them, other parameters that influence the calculation (energy offset, temperature), and any settings, and you get the result. I don't think it can be made any simpler than that.

Conclusion

I will not make a Calculation class just yet, but I will (soon) refactor Analysis1d to be cleaner.