search

icecube / pisa

Monte Carlo-based data analysis
http://icecube.github.io/pisa/
Apache License 2.0
19 stars 49 forks source link

surprising behavior from Analysis.fit_hypo #661

Open atfienberg opened 3 years ago

atfienberg commented 3 years ago

After setting a distribution maker's parameters to new Param objects, fit_hypo may discard these parameters before fitting. One example of how this could occur is this:

  1. Conduct a fit
  2. Set the distribution maker's parameters to copy.deepcopy(best_fit_params)
  3. Evaluate the distribution at the best fit parameters
  4. Fix some parameters to new values
  5. Run another fit

Under some conditions, the fit in step 5. will discard the parameters from step 4. This occurs because of the call to hypo_maker.select_params(full_param_selections). I found this behavior very surprising. Whether the fit actually uses parameters fixed before the call to fit_hypo depends on whether the user modifies parameters in place (which might work) or sets the parameters to new Param objects (which probably won't work),

philippeller commented 3 years ago

Related to #644

mvprado0 commented 3 years ago

I am finding the same problem, and with the current Pisa functions for ParamSet, there doesn't seem to be a way to make the current analysis.py code use best fit parameter starting values and set them free at the same time. All one can do if you want to keep the best fit values is fix the parameters in place as described above. A quick and simple way to fix this without messing with select_params() or ParamSet would be with a simple flag in fit_hypo that incorporates an if statement into the line hypo_maker.select_params(full_param_selections).

Something like:

def fit_hypo(self, data_dist, hypo_maker, metric, minimizer_settings, using_previous_params=False, ...): if using_previous_params==False: hypo_maker.select_params(full_param_selections)

If this is okay, I would be happy to make a PR for it.

LeanderFischer commented 3 years ago

I come across the same issue while running systematic impact tests with the high stats standard numu disappearance neutrino pipeline. Without setting recursive_minimization=True, fit_hypo is being called for every fit and for the null fits, where the physics parameters were fixed just before calling the fit, they are freed again by re-selecting the parameters, which is quite contrary to what is wanted. If instead i specify that i want the recursive minimization it directly calls fit_recursively, here, and everything is fine. Is it really necessary to re-select the parameters in fit_hypo? Not sure how other analyses are set up, but i feel for most of them the selection was already made beforehand and the hypo_maker that is passed to the fit function is using the correct selection.

P.s.: Just for my understanding, when hypo_maker.select_params is called, where does it access the original selection parameters? Should they not have been changed with the changes of hypo_maker.params? Sorry, if i'm being dense, but i'm just diving back into the intricacies of Pisa :sweat_smile: