fermi-lat / Likelihood

BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Failure on EBL models #76

Closed jballet closed 4 years ago

jballet commented 4 years ago

I reopen LK-138 on EBL models, which was marked resolved but never actually resolved. The EBL models fail to fit after calling Ts, resetting parameter values and freezing a parameter. No other model behaves that way, but all EBL models do. A test harness is attached . It will disappear after one week, but can be put back on request. Provide the reference to the 8-year livetime cube and run test_EBL.py. In short, the script saves the parameters, calls TS, recovers the parameter values, freezes a parameter, refits and calls TS again. The second fit returns very serious error messages indicating that something is inconsistent in the model. This largely prevents any complex operation. For example, fermipy is incapable of building a SED of an EblAtten source because of this.

eacharles commented 4 years ago

Ok.
This is yet another case of the problems caused by mix of the python interface layer in pyLikelihood and the fact that there are copies of the paramters in both the c++ Source objects and the c++ Likelihood object. The python layer basically duplicates the Source, Function and Parameter objects from the c++layer and really complicates the already messy bookkeeping of keeping the sources in sync with the likelihood object.

On the other hand, having the python interface layer interface layer does make it easier to debug things.

In this case the problem is when the python class SrcModel.freeze() is invoked, the command is getting passed all the way to the c++ parameter object that lives in the PowerLaw2 c++ object that defines the un-attenuated spectrum. However, it is not getting passed to the parameter that lives in the c++ likelihood object for some reason.

I'll investigate more tomorrow.

eacharles commented 4 years ago

Ok, the issue is being caused by "the LikelihoodState.restore" function, which has it's own implemenation of the Parameter class, which is is using to overwrite the parameter objects in a way that is breaking the connection between the EblAtten wrapper and the underlying class.
I'll investigate more, but I think the answer is to change the implementation to copy the values into the parameter object, rather that copying the object itself.

eacharles commented 4 years ago

Ok, I've fixed this by changing the implemenation of pyLikelihood.LikelihoodState.restore to copy the c++ parameter data members instead of making new objects, which was breaking the link between the EblAtten class and the nested PowerLaw class.

jballet commented 4 years ago

I have tested FT 1.3.1 and the offending behavior went away, as expected.