icecube / pisa

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

Copy of theta23 param causes issue #672

Closed ts4051 closed 2 years ago

ts4051 commented 3 years ago

In analysis.py the function get_separate_octant_params (used by _fit_octant) makes a copy of the theta23 Param instance (see here), and this copy is later written back to hypo_maker.params (see here).

theta23 lives in a ParamSelector object, which stores the original theta23 param value in its dict of selections. And so there is now basically a new duplicate parameter in the current params, but this will be overwritten by the original stored param in the selections dict next time ParamSelector.select_params is called (see here).

This is a problem is the user then modifies the theta23 param, since at the start of the next fit ParamSelector.select_params is called and the theta23 param is overwritten by the stored value, and thus the user's changes are lost. This breaks oscNext sensitivity scans for numu disappearance, since the user does a free fit first (resulting in the duplicate theta23 param), then fixes theta23 for the scan but at the first scan point the param is overwritten and hence it is no longer fixed, breaking the scan.

The solution to this issue is simple, simply remove the deepcopy of the original param in this line.

ts4051 commented 3 years ago

I am fixing this in my own fork now, and will make a PR ASAP

philippeller commented 3 years ago

Hey, cool! We then just need to make sure that https://github.com/icecube/pisa/blob/master/pisa/analysis/analysis.py#L327 does not change the original value

atrettin commented 2 years ago

This issue is now resolved and we have unit tests to assert this. :)