`analysis.fit_recursively()` fails when running over the Detectors class

[bleaktwig]

When creating an instance of the Detectors class using two different pipelines, the method analysis.fit_recursively(...) fails, throwing: IndexError: pop from empty list. A minimal replicating example and full traceback are included in the discussion.

[bleaktwig]

A minimal example to replicate the error (assuming wg-oscillations-fridge is installed):

import os
os.chdir("/path/to/wg-oscillations-fridge/analysis")

from pisa.analysis.analysis import Analysis
from pisa.core.detectors import Detectors
from pisa.core.pipeline import Pipeline
from pisa import ureg

LOCAL_FIT_MINUIT = {
    "method": "iminuit",
    "method_kwargs": {
        "errors": 0.1,
        "precision": 1e-14,
        "tol": 1e-2,
        "run_simplex": False,
        "run_migrad": True
    },
    "local_fit_kwargs": None
}

PHYSPARAM  = "deltam31"
TRUTHVALUE = 0.00275 * ureg.eV**2

# Setup pipelines.
pipelines: list[dict[str, Pipeline]] = [
    {
        "nu"    : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_bottom.nu.cfg"),
        "mu"    : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_bottom.mu.cfg"),
        "noise" : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_bottom.noise.cfg"),
    },
    {
        "nu"    : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_top.nu.cfg"),
        "mu"    : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_top.mu.cfg"),
        "noise" : Pipeline("oscnext_fantaustic/settings_test/pipeline/nhits_top.noise.cfg"),
    }
]

# Include detector name on each pipeline.
for pi in range(2):
    for d in pipelines[pi].values():
        d.detector_name = f"nhits{'<' if pi==0 else '>'}17.5"

# Initiate the Detectors class w/ no shared parameters.
model = Detectors([p for pdict in pipelines for p in pdict.values()],
                  shared_params=[])

# Fix all paremeters except for deltam31.
for param in model.params:
    if PHYSPARAM in param.name:
        continue
    param.is_fixed = True

# Get nominal value.
val_start = model.params[PHYSPARAM]

# Set target parameter to truth value and get true map.
model.params[PHYSPARAM] = TRUTHVALUE
map_true = model.get_outputs(return_sum=True)

# Set parameters to nominal value and fit.
model.params[PHYSPARAM] = val_start.value

# Initialize analyzer.
analysis = Analysis()

_ = analysis.fit_recursively(
    data_dist=map_true,
    hypo_maker=model,
    metric=["llh", "llh"],
    external_priors_penalty=None,
    **LOCAL_FIT_MINUIT
)

[bleaktwig]

And the full traceback:

---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
Cell In[9], line 66
     63 # Initialize analyzer.
     64 analysis = Analysis()
---> 66 _ = analysis.fit_recursively(
     67     data_dist=map_true,
     68     hypo_maker=model,
     69     metric=["llh", "llh"],
     70     external_priors_penalty=None,
     71     **LOCAL_FIT_MINUIT
     72 )

File [/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py:1253](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py#line=1252), in BasicAnalysis.fit_recursively(self, data_dist, hypo_maker, metric, external_priors_penalty, method, method_kwargs, local_fit_kwargs)
   1251 fit_function = getattr(self, f"_fit_{method}")
   1252 # Run the fit function
-> 1253 return fit_function(data_dist, hypo_maker, metric, external_priors_penalty,
   1254                     method_kwargs, local_fit_kwargs)

File [/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py:2347](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py#line=2346), in BasicAnalysis._fit_iminuit(self, data_dist, hypo_maker, metric, external_priors_penalty, method_kwargs, local_fit_kwargs)
   2345 if migrad:
   2346     logging.info("Running MIGRAD")
-> 2347     m.migrad()
   2349 end_t = time.time()
   2350 if self.pprint:
   2351     # clear the line

File [/mnt/ceph1-npx/user/bbenkel/lib/hwof-py3.12/lib/python3.12/site-packages/iminuit/minuit.py:852](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/hwof-py3.12/lib/python3.12/site-packages/iminuit/minuit.py#line=851), in Minuit.migrad(self, ncall, iterate, use_simplex)
    850 t = mutil._Timer(self._fmin)
    851 with t:
--> 852     fm = _robust_low_level_fit(
    853         self._fcn,
    854         self._last_state,
    855         replace_none(ncall, 0),
    856         self._strategy,
    857         self._tolerance,
    858         self._precision,
    859         iterate,
    860         use_simplex,
    861     )
    863 self._last_state = fm.state
    865 self._fmin = mutil.FMin(
    866     fm,
    867     "Migrad",
   (...)
    874     t.value,
    875 )

File [/mnt/ceph1-npx/user/bbenkel/lib/hwof-py3.12/lib/python3.12/site-packages/iminuit/minuit.py:2863](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/hwof-py3.12/lib/python3.12/site-packages/iminuit/minuit.py#line=2862), in _robust_low_level_fit(fcn, state, ncall, strategy, tolerance, precision, iterate, use_simplex)
   2861 if precision is not None:
   2862     migrad.precision = precision
-> 2863 fm = migrad(ncall, tolerance)
   2864 strategy = MnStrategy(2)
   2865 migrad = MnMigrad(fcn, fm.state, strategy)

File [/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py:2303](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py#line=2302), in BasicAnalysis._fit_iminuit.<locals>.loss_func(x)
   2301     logging.warning(f"Minuit tried evaluating at invalid parameters: {x}")
   2302     return np.nan
-> 2303 return self._minimizer_callable(x, *args)

File [/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py:2763](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/analysis/analysis.py#line=2762), in BasicAnalysis._minimizer_callable(self, scaled_param_vals, hypo_maker, data_dist, metric, counter, fit_history, flip_x0, external_priors_penalty)
   2761 scaled_param_vals = np.where(flip_x0, 1 - scaled_param_vals, scaled_param_vals)
   2762 # Set param values from the scaled versions the minimizer works with
-> 2763 hypo_maker._set_rescaled_free_params(scaled_param_vals) # pylint: disable=protected-access
   2764 if hypo_maker.__class__.__name__ == "Detectors":
   2765     update_param_values_detector(hypo_maker, hypo_maker.params.free) #updates values for ALL detectors

File [/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/core/detectors.py:340](http://127.0.0.1:8251/mnt/ceph1-npx/user/bbenkel/lib/pisa/pisa/core/detectors.py#line=339), in Detectors._set_rescaled_free_params(self, rvalues)
    338         rp = []
    339         for j in range(len(d.params.free)):
--> 340             rp.append(rvalues.pop(0))
    341         d._set_rescaled_free_params(rp)
    343 else:

IndexError: pop from empty list

[JanWeldert]

The problem is that after changing a parameter in model.params, you have to run update_param_values_detector (from pisa.analysis.analysis import update_param_values_detector) to actually update the params in all internal distribution makers. In your case, running update_param_values_detector(model, model.params, update_is_fixed=True) after changing one (or more) params should fix the problem. In a fit this is done, but since you changed the param values interactively, you also have to run update_param_values_detector here. The reason why we have to do it is connected to the way the detectors class internally stores the params. I know it is not ideal. I will think about a better way handling this, but for know using update_param_values_detector should fix your issue.

[bleaktwig]

Thank you! That fixed the problem. I agree that it would be nice for the class to handle parameter changes better, but since there is a work-around I'll label this as enhancement :)