Fitter subclasses no longer work

[markbandstra]

The subclasses of Fitter, namely FitterGausGaus, FitterGausGausLine, and FitterGausGausExp, no longer work. The failure is

Traceback (most recent call last):
  File "analyze_data.py", line 124, in <module>
    fitter = bq.fitting.FitterGaussGaussExp(
  File "becquerel/core/fitting.py", line 356, in __init__
    self._make_model(model)
  File "becquerel/core/fitting.py", line 536, in _make_model
    model_translated = [self._translate_model(m) for m in model]
TypeError: 'NoneType' object is not iterable

The problem seems to be that they no longer follow the Fitter API correctly. It seems that their make_model methods do not get called, instead the base class's _make_model method is called. The fix seems to be to change their make_model methods to be _make_model, but they also need to take a dummy model argument. That is, instead of

    def make_model(self):

these need to be

    def _make_model(self, model):

and then they seem to work again.

It seems that the primary utility of these classes is their _guess_param_defaults methods, which are very useful when, e.g., multiple gaussians are fit and you want to start them in different portions of the ROI.

[jvavrek]

@markbandstra do you mind finding the last commit where they did work?

[cosama]

@jvavrek I suspect they never did. I think they were an artifact of before we implemented the simplified model creation through a string list. At that point we changed how models are handled and realized that we only really need a single generic Fitter class. I think these models should be created through:

fit = Fitter(GaussModel("gauss1") + GaussModel("gauss2"))

etc.

[markbandstra]

Looks like they worked until 0436e7ce

In that refactoring, Fitter.make_model is changed to _make_model, and various Fitter subclasses like FitterGauss and FitterGaussLine are removed, presumably because they were made obsolete. My hunch is that the others remained because they involve multiple gaussians and so their _guess_param_defaults methods are valuable.

[cosama]

@markbandstra Exactly. I think they remained because we haven't moved them to GaussGaussModel, etc, while the others are available as models now. and it must have been on one of the lists in the fit merge request, but got lost at some point.

It should be worth thinking about how to define the initial values for such a model though. Maybe parameters could be directly passed in with lower and upper bounds to the constructor of GaussModel(), etc.

I think it is almost impossible to define a generic two, or three gaussian function, the centroids need to be very clearly defined.

[markbandstra]

@cosama we could have _guess_param_defaults detect if there are identical models of any type and add dispersion to any centroid-like (centroidoid?) parameters. Or always call guess with center_ratio=(1 + j)/(1 + n) for the jth gaussian if there are n gaussians.