Run3-gex136D Try to take care of compilation warnings in TopQuarkAnalysis/TopKinFitter

[bsunanda]

PR description:

Try to take care of compilation warnings in TopQuarkAnalysis/TopKinFitter

PR validation:

Use the runTheMatrix test workflows

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Nothing special

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38661/30950

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

• TopQuarkAnalysis/TopKinFitter (analysis)

@cmsbuild can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[bsunanda]

@cmsbuild Please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-944649/26091/summary.html
COMMIT: dada1f1
CMSSW: CMSSW_12_5_X_2022-07-08-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38661/26091/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 50
• DQMHistoTests: Total histograms compared: 3655935
• DQMHistoTests: Total failures: 8
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3655905
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
• Checked 208 log files, 45 edm output root files, 50 DQM output files
• TriggerResults: no differences found

[perrotta]

+1

[perrotta]

merge

[makortel]

Sorry to note only now, but I'm concerned that the migration done here might be too simple. I left TtFullLepKinSolutionProducer as a legacy module in #36692 because the code was using potentially thread-unsafe constructs (see the description of ##36692 for more details).

[perrotta]

Sorry to note only now, but I'm concerned that the migration done here might be too simple. I left TtFullLepKinSolutionProducer as a legacy module in #36692 because the code was using potentially thread-unsafe constructs (see the description of ##36692 for more details).

Thank you Matti for the follow up.
What do you suggest now, then? To revert the changes to TtFullLepKinSolutionProducer (and only those for that file)?

[makortel]

It seems to me that this module is only ran in one unit test

cmssw/TopQuarkAnalysis/TopKinFitter/test/ttFullLepKinSolutionProducer_cfg.py

Line 41 in d46ee62

process.load("TopQuarkAnalysis.TopKinFitter.TtFullLepKinSolutionProducer_cfi")

and with one thread only (which could partially explain why we haven't observed any problems).

Personally I'm also wondering if this code is being really used for anything else than the aforementioned test.

@pcanal Could you comment how thread safe TF2::Eval() is? In particular, can different objects (that may have the same name) be used safely from different threads? With TF1 we explicitly asked to not add the function to the global list in order to achieve such "thread-friendliness" (#18636), but I did not see similar option for TF2.

[pcanal]

TF2 inherits from TF1 so you should apply the same pattern to it.

[perrotta]

It seems to me that this module is only ran in one unit test

cmssw/TopQuarkAnalysis/TopKinFitter/test/ttFullLepKinSolutionProducer_cfg.py

Line 41 in d46ee62

process.load("TopQuarkAnalysis.TopKinFitter.TtFullLepKinSolutionProducer_cfi")

and with one thread only (which could partially explain why we haven't observed any problems).
Personally I'm also wondering if this code is being really used for anything else than the aforementioned test.

@kfjack @jkiesele you can probably check and answer whether the TtFullLepKinSolutionProducer (and possibly also other modules in TopQuarkAnalysis/TopKinFitter) are still in use for the TOP quark analyses or not.

If they are still used, it would be appreciated if someone from the TOP PAG could take care of maintaining and keeping them up to date, at least from time to time.

Otherwise, if that and possibly other producers here are not used any more, could you please list which one of those producers can be removed from the release, thus relieving the code experts from uselessly spend time in maintaining them?

Thank you!

[makortel]

TF2 inherits from TF1 so you should apply the same pattern to it.

I see in https://root.cern/doc/master/classTF1.html the EAddToList can be given to TF1 only via constructor argument. For TF2 (in https://root.cern/doc/master/classTF2.html) I don't see that type at all.

[pcanal]

Indeed, that is missing :( ... See the new issue: root-project/root#11460

Note that you can work around this (at the expense of some scalability) by doing something like:

{
    R__LOCKGUARD(gROOTMutex);
    f2 = new TF2(....)
    gROOT->GetListOfFunctions()->Remove(f2);
}