search

key4hep / EDM4hep

Generic event data model for HEP collider experiments
https://cern.ch/edm4hep
Apache License 2.0
24 stars 35 forks source link

Vertex `probability` member should be `ndf` #318

Closed tmadlener closed 3 weeks ago

tmadlener commented 1 month ago

The Vertex currently has members chi2 and probability. The much more natural choice in this case would be to have ndf instead of probability, since that gives the information about the fit quality on a lower level from which higher level quantities, such as probability can be easily calculated.

https://github.com/key4hep/EDM4hep/blob/bd9f45039149781a9c54799a6d03df56b1b5ed49/edm4hep.yaml#L551-L557

jmcarcell commented 1 month ago

I think other than in the converter probability is never used in Key4hep and others and this change makes sense. Also going from chi2 and ndf to probability is trivial while the other way not so much. For tracks we already have ndf.

Thinking about all the fits there is also Hyptothesis that has chi2 but no probability nor ndf. In case these proliferate (if it's only for tracks and vertexes then maybe it's not worth it), we may want to introduce something like a FitResult that would have chi2 and ndf and provide some utilities to get the p value if there is a need for it.

tmadlener commented 1 month ago

Yeah, for Hypothesis I am also not entirely sure what the chi2 is exactly. See also one point on this #323

I agree with the rest. I would be in favor of keeping chi2 and ndf for now (mainly because it is less work ;) )

gaede commented 1 month ago

We (BH, JMC, FG) agree to replace probability w/ ndf - and suggest to not introduce a component now.