[10.6.X] fix the cms EDModule type of GenParticleMatchMerger

[mmusich]

backport of #37613

PR description:

It appears that CollectionAdder

cmssw/CommonTools/UtilAlgos/interface/CollectionAdder.h

Line 17 in 837fd56

class CollectionAdder : public edm::global::EDProducer<> {

which is the underlying module type of

cmssw/PhysicsTools/HepMCCandAlgos/plugins/GenParticleMatchMerger.cc

Line 4 in 6d2f660

typedef CollectionAdder<reco::GenParticleMatch> GenParticleMatchMerger;

is an EDProducer, but it was declared in the configuration as an EDFilter:

cmssw/SimTracker/TrackAssociation/python/allTrackMCMatch_cfi.py

Line 3 in 837fd56

allTrackMCMatch = cms.EDFilter("GenParticleMatchMerger",

leading to runtime errors of the type:

----- Begin Fatal Exception 19-Apr-2022 00:54:18 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=GenParticleMatchMerger label='allTrackMCMatch'
Exception Message:
The base type in the python configuration is EDFilter, but the base type
for the module's C++ class is EDProducer. Please fix the configuration.
It must use the same base type as the C++ class.
----- End Fatal Exception -------------------------------------------------

this is trivially fixed here.
In addition in commit f68a4df, I take care of some other mismatched types in the configuration.
The parameter associator of MCTrackMatcher should be a cms.string and not a cms.InputTag:

cmssw/SimTracker/TrackAssociation/plugins/MCTrackMatcher.cc

Line 44 in 9583b99

: associator_(consumes<reco::TrackToTrackingParticleAssociator>(p.getParameter<string>("associator"))),

Finally in the same commit, the value of the variable associator in Tracker/TrackAssociation/python/trackMCMatch_cfi.py is corrected such that it is trackAssociatorByHits and not TrackAssociatorByHits

PR validation:

Private scripts.

if this PR is a backport please specify the original PR and why you need to backport that PR:

verbatim backport of #37613, useful for Ultra-Legacy analysis

cc: @tvami

[cmsbuild]

A new Pull Request was created by @mmusich (Marco Musich) for CMSSW_10_6_X.

It involves the following packages:

• SimTracker/TrackAssociation (simulation)

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@mtosi, @abbiendi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @jhgoh, @mmusich, @threus, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

• Backported from fix the cms EDModule type of GenParticleMatchMerger #37613

[mmusich]

type bug-fix

[mmusich]

@cmsbuild, please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3a74d9/24084/summary.html
COMMIT: b1a049e
CMSSW: CMSSW_10_6_X_2022-04-17-0000/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37641/24084/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: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 3215686
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3215351
• DQMHistoTests: Total skipped: 334
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 143 log files, 29 edm output root files, 35 DQM output files
• TriggerResults: no differences found

[civanch]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_4_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[perrotta]

@mmusich the original PR was merged in master (i.e. 12_4_X), and you are proposing a backport to 10_6 here. What about the intermediate releases? Could you please prepare similar backports also for the intermediate cycles also (i.e. at least 12_2 and 12_3)?

By the way: these configs are only used for track validation, isn't it? I.e. they do not affect simulation (otherwise it would have crashed, I'd say...)

[tvami]

Hi @perrotta
I wanted to create a (private) simulation that can be used to make sure there are no fake tracks in the sample, and this needs the allTrackMCMatch collection. And it crashed indeed, thus the PR. No official production uses this so far, although I think it would be useful... On the other hand, I dont expect any production with 12_2_X or 12_3_X that will need this fix.

[mmusich]

What about the intermediate releases? Could you please prepare similar backports also for the intermediate cycles also (i.e. at least 12_2 and 12_3)?

I can, but as far as I can tell they would serve no purpose, other than making the code formally correct.

these configs are only used for track validation, isn't it? I.e. they do not affect simulation (otherwise it would have crashed, I'd say...)

they do affect the event products saved in simulation when certain customizations are used (see e.g. this commit).
This can be useful when preparing private MC with track/MC truth matching, @tvami can elaborate further as he's the one interested in it.
The default behaviour is indeed not affected.

[perrotta]

@tvami @mmusich thank you for your answer.
I understand that this is only needed for some private sim production. And as such it can also be merged only in that particular release, for that particular purpose.
Still, if possible I would prefere to avoid having "holes" in the release cycles, i.e. something which is fixed in a given old release, it is fixed from one release onwards, but it is not in the intermediate strill used releases.
Since the fix is quite logical, and simple to implement, I'd suggest to merge it also in the intermediate cycles still used for productions and analysis:y I'd therefore limit it on 12_2 and 12_3, letting all other release cycles aside. I agree that it could also be useless at the end...

[perrotta]

+1

• Let have it merged anyhow in 10_6, and I let you decide whether to prepare also the other backports

[mmusich]

Still, if possible I would prefere to avoid having "holes" in the release cycles, i.e. something which is fixed in a given old release, it is fixed from one release onwards, but it is not in the intermediate strill used releases.

No problem, I'll prepare them tomorrow.

[mmusich]

No problem, I'll prepare them tomorrow.

submitted:

• 12.3.X: [12.3.X] fix the cms EDModule type of GenParticleMatchMerger #37652
• 12.2.X: [12.2.X] fix the cms EDModule type of GenParticleMatchMerger #37653

[perrotta]

No problem, I'll prepare them tomorrow.

submitted:

• 12.3.X: [12.3.X] fix the cms EDModule type of GenParticleMatchMerger #37652
• 12.2.X: [12.2.X] fix the cms EDModule type of GenParticleMatchMerger #37653

Thank you Marco!