ADR Suggestion Fitting Using Third-Party Minimizers – Design Review & Clarifications

[AndrewSazonov]

We currently have an ADR titled "Fitting using third-party minimizers" (#9).

This ADR was created before we established the formal "Protocol for Creating ADRs", so it doesn't have an associated discussion thread suggesting it as a new ADR.

Since a few new questions and concerns have surfaced regarding the current design, I am creating this ADR suggestion to discuss and clarify them. Once we have consensus, we can update the original ADR, if necessary.

Proposed API Improvements

The existing ADR states:

MinimizerFactory - Single class / function

However, it doesn’t explain why the current implementation uses a function rather than a class.
Additionally, it doesn’t provide reasoning for why AvailableMinimizers is implemented as a separate class.

So, we have:

• A factory function located in
  easyscience/fitting/minimizers/factory.py
• An AvailableMinimizers class located in
  easyscience/fitting/available_minimizers.py

This structure feels unnecessarily complex and potentially confusing.
Below is a simplified class diagram illustrating the current setup:

classDiagram

    %%%%%%%%%%%%%%%%%%%%%%%%%
    %% easyscience package %%
    %%%%%%%%%%%%%%%%%%%%%%%%%

        class MultiFitter:::fitter {
        }

        %% Rename to SingleFitter?
        class Fitter:::fitter {
            ...
            %% Rename to _fitter: fitter_cls?
            _minimizer: minimizer_cls
            ...
        }

        %% Rename to AvailableFitters?
        %% Move to FitterFactory class instead?
        class AvailableMinimizers {
            ...
        }

        %% Rename to AvailableFitter?
        %% Move to FitterFactory class instead?
        class AvailableMinimizer {
        <<dataclass>>
        }

        %% Move to FitterFactory class instead?
        class factory:::factory {
            factory(...) minimizer_cls
        }

        %% Rename to LmfitFitter?
        class LMFit:::wrapper {
        }

        %% Rename to BumpsFitter?
        class Bumps:::wrapper {
        }

        %% Rename to DfolsFitter?
        class DFO:::wrapper {
        }

        %% Rename to FitterBase?
        class MinimizerBase:::baseclass {
            <<abstract>>
            /property/ all_constraints
            /property/ enum
            /property/ name
            _get_method_kwargs(...)
            _prepare_parameters(...)
            _fit_function(...)
            _create_signature(...)$
            _error_from_jacobian(...)$
            fit_constraints()
            set_fit_constraint(...)
            add_fit_constraint(...)
            remove_fit_constraint(...)
            evaluate(...)
            convert_to_pars_obj(...)*
            fit(...)*
            supported_methods()$
            all_methods()$
            convert_to_par_object(...)$
        }

    %%%%%%%%%%%%%%%
    %% RELATIONS %%
    %%%%%%%%%%%%%%%

    %% Refinement/Fitting/Minimization/Optimization %%

    DiffractionFitter *-- MultiFitter : contains

    MultiFitter --|> Fitter : inherit

    Fitter ..> factory : uses

    Fitter *-- LMFit : contains
    Fitter *-- Bumps : contains
    Fitter *-- DFO : contains

    factory ..> LMFit : creates
    factory ..> Bumps : creates
    factory ..> DFO : creates

    LMFit --|> MinimizerBase : inherit
    Bumps --|> MinimizerBase : inherit
    DFO --|> MinimizerBase : inherit

    Fitter .. AvailableMinimizers : uses
    factory .. AvailableMinimizers : uses
    LMFit .. AvailableMinimizers : uses
    Bumps .. AvailableMinimizers : uses
    DFO .. AvailableMinimizers : uses
    MinimizerBase .. AvailableMinimizers : uses

    AvailableMinimizers --|> AvailableMinimizer : inherit

    %%%%%%%%%%%%%
    %% STYLING %%
    %%%%%%%%%%%%%

    %% Core Containers (Project, Analysis)
    classDef container fill:#546E7A,stroke:#263238,stroke-width:1px,color:#FFFFFF;

    %% Diffraction Fitter
    classDef fitter fill:#FBC02D,stroke:#F57F17,stroke-width:1px,color:#000000;

    %% Diffraction Calculator (Core Calculation Logic)
    classDef calculator fill:#FBC02D,stroke:#F57F17,stroke-width:1px,color:#000000;

    %% Factories (Factory classes in Minimization/Calculation)
    classDef factory fill:#2E7D32,stroke:#1B5E20,stroke-width:1px,color:#FFFFFF;

    %% Wrappers (Crysfml, Cryspy, Pdffit Calculators)
    classDef wrapper fill:#FB8C00,stroke:#E65100,stroke-width:1px,color:#FFFFFF;

    %% Abstract Base Classes (MinimizerBase, CalculatorBase)
    classDef baseclass fill:#3949AB,stroke:#1A237E,stroke-width:1px,color:#FFFFFF;

Loading

Proposal

I suggest we discuss whether this design can be improved by consolidating the functionality into a single MinimizerFactory class, which would:

• Handle the creation of minimizers
• Maintain information about available minimizers

This could simplify both the internal design and the user-facing API.

User API (No Change Proposed)

Regardless of whether we keep the existing design or adopt a new, unified design, users should not need to access the factory directly.
We have already agreed that the Analysis object is responsible for minimization, so the API for users should remain simple and intuitive:

for mini in project.analysis.available_minimizers:
    print(mini)

project.analysis.current_minimizer = 'bumps_lm'

Naming Convention Clarification

There is currently a mix of fit-based and minimize-based names in the codebase and documentation.
This inconsistency can lead to confusion.

We should clarify when and where to use:

• fit, Fitter, available_fitters, etc.
  versus
• minimize, Minimizer, available_minimizers, etc.

I propose we document the chosen convention explicitly as part of this ADR.

[AndrewSazonov]

Related resources and minimization engines we use:

scipy.optimize: nonlinear least-squares and curve fitting
https://docs.scipy.org/doc/scipy/reference/optimize.html
https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize.html
https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.curve_fit.html

Lmfit: non-linear optimization and curve fitting
https://lmfit.github.io/lmfit-py/index.html
https://lmfit.github.io/lmfit-py/fitting.html#module-lmfit.minimizer
https://lmfit.github.io/lmfit-py/model.html#lmfit.model.Model.fit

Bumps: curve fitting and uncertainty analysis from a Bayesian perspective
https://bumps.readthedocs.io/en/latest/getting_started/index.html
https://bumps.readthedocs.io/en/latest/guide/fitting.html
https://bumps.readthedocs.io/en/latest/guide/optimizer.html

DFO-LS: derivative-free optimizer for least-squares minimization
https://numericalalgorithmsgroup.github.io/dfols/build/html/index.html

[damskii9992]

The factory and AvailableMinimizers

As far as I can see, the reason why AvailableMinimizers is implemented as a separate class, is in order to use the DataClass decorator to automatically add nice formatting dunder methods.
I think this makes a lot of sense as the user will use the AvailableMinimizers directly to query what methods are available.
The instantiated object of AvailableMinimizers could however be a property of the factory, if it was converted to a class, but I'm not sure if that actually makes anything easier at all.
Where would that class belong? Certainly not to the Analysis object, what goes there would be the actual Minimizer object produced by the factory. I.e. whenever the Analysis class is changing or updating its minimizer, it queries the factory in order to get a new minimizer.
We also don't want to save the factory when we save the project, as it has nothing to do with the user.
Finally we can easily have the same user API not matter our implementation, simply make the method in:

for mini in project.analysis.available_minimizers:
    print(mini)

call the DataClass.

So in summary, I'm not in favor of changing the current design, I think it makes a lot of sense. What is gained in terms of code structure, is lost many times over in terms of functionality and code complexity elsewhere.

Naming convention

In some way I actually think the current naming convention makes sense. Anything named Minimizer deals with the minimizing backends, i.e. lmfit, bumps etc. whereas anything names Fitter is our code dealing only with our classes and structures.
So currently a Fitter will always have a Minimizer, because the Minimizer is responsible for translating our syntax to the backends syntax.
I do see your point of it appearing confusing though. Even though it is properly explained in #9.

We could rename the current Minimizers to Wrappers instead, or FitWrapper (LmfitFitWrapper? LmfitMinimizerWrapper?) but that leads to quite long names. Not sure what else can be done.