Get by token

[rfriese]

Dear Kappa users,

I changed all active getByLabel calls with getByToken. This speeds up the Kappa runtime by about 20 - 30% according to the automatic runtime monitoring. It's also more fail-safe and the modern way to go.

The "filterSummary" collection won't be there after this PR, which is not a bad thing because it has been empty for quite some time now and at least in the HiggsAnalysis we're not using it anyway.

Some code parts have been excluded from compilation because they have been inactive at least since the introduction of CMSSW76X.

You're invited to test this PR. Please report problems in the ongoing week, I'll merge on monday.

[greyxray]

Dear Raphael,
thanks for the development!
I will have some more questions comments later, for now to fix the kappa tests could you also add to this PR a commit where the checout script is downloaded like it was done here (since the branch CMSSW9 is not there anymore): https://github.com/KappaAnalysis/Kappa/blame/c58f9dd66c4a3e013f7f03fe67b189c01126edfe/test_build.sh#L172

[greyxray]

Dear Raphael,
in this commit TargetSetupMap is storing TargetSetupMapContent with additional field for the Token storage. But is this field later used anywhere?

[greyxray]

Dear Raphael,
in this commit can the beamSpotSource and pfTauCollection be removed now safely?

[greyxray]

Dear Raphael,

did you mean to use here VertexCollectionSourse? I do not quite yet understand the difference for the Tag and Token but something similar is in KTowerProduce commit

[greyxray]

Dear Raphael,
for the KJetProducer this seems never be executed now, or no? Could you point me where the removed in this commit functionality is written in a more flexible way? Or it is not used at all?

[rfriese]

Hi Olena,

thank you a lot for the feedback.

the targetSetupMap has now three instead of two members, they are accessed
https://github.com/KappaAnalysis/Kappa/blob/getByToken/Producers/interface/KBaseMultiProducer.h#L38-L39
https://github.com/KappaAnalysis/Kappa/blob/getByToken/Producers/interface/KBaseMultiProducer.h#L47

What do you mean if the beamSpotSource and pfTauCollection can be removed? From where?

The vertexCollection(Source) is the same for KTower and KBasicTauProducer; they just have different names. There's a lot of legacy in the code depending on who wrote the modules, so "vertexCollectionSource" and "srcPVs" are different names for doing the same thing.

What I removed from the KJetProducer wasn't used in Run2 at all - same thing about the lacking consumes call. It also contained hard-coded names and lacked configurability. This is a no-go, so if someone might want to use it in the future, one has to rewrite this part properly anyway. The Jet collection itself is independent of this part that I removed and can still be used.

[greyxray]

Dear Raphael,

thank you for the explanations.

You are right, my comment about beamSpotSource and pfTauCollection is completely invalid, I fooled myself, sorry for the confusion.

What concerns KBasicTauProducer, I understood it in the way that when the producer's constructor is called the initial parameter here is set as defined in the python configure file, but later in the constructor the token is controlled with a constant string - the same as the default value of the VertexCollectionSource parameter. I don't see VertexCollectionSource to be used in the file later so I concluded that you wanted to use it here instead of the "vertexcollection" string.

[rfriese]

Ah, I see. Getting the parameter again from the parameterset is not necessary, you're right. I'll fix this before doing the merge. Thank you!

[thomas-mueller]

I would like to request that this PR is not merged before the issue related to the previous PR is solved. To increase confidence in these changes it could also help to work on the travis tests first. Thanks.

[greyxray]

Hi Thomas!
readind your previous comments, do you want to include Artus tests to travis or what exactly did you mean?
Cheers

[thomas-mueller]

No, first the Kappa tests should work (but you, Olena, are not responsible for all tests!). I think it is impossible or too complicated to check the interplay between Kappa and Artus via travis. And we should keep in mind, that travis does not free developers from checking their changes manually as good as possible.

(There are also travis tests in Artus, but currently they do not work. I do not know the status/reasons there.)

[rfriese]

PR closed. It causes a dictionary change, so the PR will be repeated to the dictchanges branch