Deployment of likelihood fits in DQMGenericClient etc

[makortel]

This issue is to track the deployment of likelihood fits and Minuit2 as a followup to the discussion at the end of #43106 .

Short history: ROOT 6.30 switched the default minimizer from Minuit to Minuit2. With the chi2 fits in e.g. DQMGenericClient this change resulted in VariableMetricBuilder Initial matrix not pos.def. exceptions that were reported in #42979). The suggested fix was to change the fits to likelihood instead of chi2. This change was done in #43106 for DQMGenericClient, that resulted later physics validation failures as reported in #43106 (comment) .

The suggestion is that in parallel of the workarounds to continue using ROOT 6.30 (discussed in #43106 (comment) onwards)

• @cms-sw/tracking-pog-l2 a d @cms-sw/muon-pog-l2 would look into restricting fit ranges as suggested in Use likelihood fit instead of chi-square fit in DQMServices slice fits #43106 (comment)
• then the DQMGenericClient would be changed back to use likelihood fits
• then the Minuit2 would be changed back to be the default (no longer needed as we stayed with Minuit2 with Make Minuit2 error on non-posdef initial matrix an error message instead of an exception #43726)

I suppose each step would require some level of validation.

[makortel]

assign pdmv, dqm

[cmsbuild]

New categories assigned: pdmv,dqm

@AdrianoDee,@sunilUIET,@miquork,@rvenditti,@syuvivida,@tjavaid,@nothingface0,@antoniovagnerini you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @makortel Matti Kortelainen.

@makortel, @Dr15Jones, @antoniovilela, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[guitargeek]

Note that the error with fit about the Hesse Matrix not being positive definite was already a problem with the old Minuit (as I figures out by re-fitting the DQM histograms standalone). It's just that they got not printed via the ROOT Error logging, which is converted to exceptions by CMSSW.

In my opinion, the appropriate hotfix would therefore be what @Dr15Jones suggested here, ignoring such exceptions:
#42979 (comment)

[makortel]

If printing out the error should be the only difference between Minuit and Minuit2, then I agree that turning the ROOT error message to a printout instead of an exception would be better than going back to Minuit. I believe we should still revert the fit method from likelihood to chi2 until the fit ranges have been adjusted.

Another question is then if we should leave the VariableMetricBuilder Initial matrix not pos.def. error message as non-fatal in the long term as well? If the cause of the message is "just" bad input, is that good-enough reason to terminate the job?

[makortel]

In my opinion, the appropriate hotfix would therefore be what @Dr15Jones suggested here, ignoring such exceptions:
#42979 (comment)

Anyway, this is now done in #43726

[smuzaffar]

After #43726, I guess we should revert

• Use likelihood fit instead of chi-square fit in DQMServices slice fits #43106
• Use likelihood fit instead of chi-square fit for PFJetDQMPostProcessor plugin #43170
• Use likelihood fit instead of chi-square fit in DQMGenericClient::limitedFit #43490
• Use likelihood fit instead of chi-square fit in PVValidation #43588

[makortel]

I guess we should revert

• Use likelihood fit instead of chi-square fit in DQMServices slice fits #43106
• Use likelihood fit instead of chi-square fit for PFJetDQMPostProcessor plugin #43170
• Use likelihood fit instead of chi-square fit in DQMGenericClient::limitedFit #43490
• Use likelihood fit instead of chi-square fit in PVValidation #43588

@smuzaffar Are you planning to revert these PRs? Or what is the plan? (sorry I missed ORP)

[smuzaffar]

@makortel , there was no discussion about these during ORP but I am in favor of reverting these. I am opening a revert PR now