ComPWA / ComPWA-legacy

[deprecated] C++ back-end for the Common Partial Wave Analysis framework
https://compwa.github.io/legacy
Other
9 stars 9 forks source link

Check parameter bounds #254

Closed weidenka closed 7 months ago

weidenka commented 4 years ago

The class ComPWA::FitParameter has a member variable Bounds but it is not checked if a value outside those bounds is set. Also in FitFraction::calculateIntensityIntegralData() bounds are not checked which leads in my case the an exceptions since one of the ComPWA::FunctionTree::FitParameter is beyond its boundaries.

spflueger commented 4 years ago

So @weidenka and me just had a small discussion about this. I want to share this with everyone, because we still need a conclusion for this.

My view of this parameter bound check would be that the Intensity defines a boundary check itself, similar to the check in the kinematics class. So the model knows what its parameter domain is, which is defined by the underlying physics. Therefore if the current set of parameters is outside of the parameter domain, then an exception is thrown. This functionality would still be needed. In my opinion this feature is atm not crucial, since the Intensity woulds return nan or inf and the fit would fail anyways. However the user would have to check himself what went wrong.

On the contrary to the Parameters of the Intensity, the FitParameters are initialization settings for the optimization. So values of the initial parameters, if a parameter is fixed or not, and potentially some boundaries for parameters respected by the optimizer. These boundaries are independent of the the parameter domains from the model. They simply control the optimizer that the parameter will not go outside this boundary. If this boundary can be smaller and could even be bigger than the physical one. If its bigger and the parameters are outside of the domain, then an exception would be thrown an the program would notify the user about this problem.

A few months ago these two things, Parameter of the Intensity and the FitParameters, were the same thing! This is not a good design (at least in my opinion) and thats why I removed this coupling. And things are like I described above.

What are your thoughts on this @redeboer @weidenka @zhang-jingqing ?

I would also be interested what a user expects and what behavior or steering possibilities a user would like to have? (This would be a question for Sebastian and Leonard)

weidenka commented 4 years ago

I'm fine with a design in which physical parameter boundaries (e.g. positive width) are checked by the Intensity itself (yet, I'm a bit worried how this can be implemented in a readable piece of code). As long as such functionality does not exist, to my opinion FunctionTree::FitParameter should check its boundaries, as it currently does.