Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consumes migration for L1GtUtils #9055

Merged
merged 9 commits into from May 22, 2015
Merged

Conversation

wddgit
Copy link
Contributor

@wddgit wddgit commented May 12, 2015

This modifies L1GtUtils to call the consumes function
for all the products it gets. It also modifies the numerous
modules that use L1GtUtils to work with these changes.

The L1GtUtils constructor is modified and its arguments
determine its behavior. This PR preserves the previous
behavior where it is possible to have L1GtUtils look for
the products by type without specifying the InputTags in
the configuration. This makes the migration of the
modules easier as configurations will not need modification
to add the InputTag parameters. It is also possible to pass
the InputTags into the constructor as arguments, which was
also an option in the earlier versions. And I added the the new
possibility to set the InputTags in the configuration as well.

All modules that use L1GtUtils will also have to remove InputTags
where they were being passed in at points other than the constructor.
They have to be set in the constructor now. This has to be decided
earlier and the module can no longer decide the InputTag when processing
the event or run.

The new L1GtUtilsHelper class encapsulates most of the logic of the
input tag selection and holds the EDGetTokens. It also registers the
callback function (its operator() function) which is called for all products
in the ProductRegistry if the InputTags are not specified in the configuration
or as arguments to the constructor.

wddgit added 8 commits May 1, 2015 16:24
This modifies L1GtUtils to call the consumes function
for all the products it gets. Also the L1GtAnalyzer
module has consumes calls added. They also use
getByToken instead of getByLabel now.

I did not yet modify all the modules that use L1GtUtils
yet. This will be done next in a separate commit.

Modules will need to use a different L1GtUtils
constructor and the arguments determine the behavior.
This preserves the previous behavior where it is
possible to have L1GtUtils look for the products
by type without specifying the InputTags in the
configuration. This will make the migration of the
modules easier as configurations will not need
modification to add the InputTag parameters.
It is also possible to pass the InputTags into the
constructor as arguments which was also an
option in the earlier versions. And I added the
the new possibility to set the InputTags in the
configuration as well.

All modules that use L1GtUtils will also have to
remove InputTags where they were being passed in
at points other than the constructor. They have to
be set in the constructor now. This has to
be decided earlier and the module can no longer
decide the InputTag when processing the event or
run.

The new L1GtUtilsHelper class encapsulates most
of the logic of the input tag selection and
holds the EDGetTokens.
The previous commit added the consumes calls
for L1GtUtils. This commit modifies the things
that depend on L1GtUtils so that they still
work with the modified L1GtUtils.

New arguments were added to the L1GtUtils constructor.
Specifying InputTags to other functions in L1GtUtils
is no longer allowed.

Some modules directly use L1GtUtils and some use it
through a helper class. The most significant of the
helper classes was HLTConfigProvider. In that case,
the four functions that required its L1GtUtils member
were split into a new helper class HLTPrescaleProvider.
Modules that did not use those functions does not
need modification, but those that did now have to
use HLTPrescaleProvider.
An earlier commit added the consumes calls
for L1GtUtils. This commit modifies the things
that depend on L1GtUtils so that they still
work with the modified L1GtUtils.
Fix logic in earlier commit (not submitted in
a pull request yet). You cannot call getByToken
with an uninitialized token.

Also pick the first product found instead of the
last so we do not declare we consume multiple
products (should not make any difference if there
is only one).
Fix a couple minor problems in earlier commits
which were not submitted in a PR yet.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @wddgit (W. David Dagenhart) for CMSSW_7_5_X.

Consumes migration for L1GtUtils

It involves the following packages:

Calibration/HcalCalibAlgos
Calibration/IsolatedParticles
CommonTools/TriggerUtils
DPGAnalysis/Skims
DQM/L1TMonitor
DQM/Physics
DQM/SiStripMonitorCluster
DQM/SiStripMonitorTrack
DQM/TrackerCommon
DQM/TrackerMonitorTrack
DQM/TrackingMonitor
DQM/TrigXMonitor
DQMOffline/JetMET
DQMOffline/L1Trigger
DQMOffline/Muon
HLTrigger/HLTanalyzers
HLTrigger/HLTcore
L1Trigger/GlobalTriggerAnalyzer
L1TriggerConfig/L1GtConfigProducers
PhysicsTools/PatAlgos
PhysicsTools/TagAndProbe
Validation/RecoTau
Validation/RecoVertex

@perrotta, @cmsbuild, @diguida, @cvuosalo, @boudoul, @fwyzard, @franzoni, @cerminar, @monttj, @Martin-Grunewald, @srimanob, @nclopezo, @deguio, @slava77, @vadler, @mmusich, @mulhearn, @danduggan can you please review it and eventually sign? Thanks.
@TaiSakuma, @abbiendi, @rappoccio, @Martin-Grunewald, @threus, @venturia, @mmarionncern, @battibass, @makortel, @acaudron, @jhgoh, @jdolen, @ferencek, @cerati, @trocino, @rociovilar, @barvic, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @nhanvtran, @schoef, @dgulhan, @imarches, @appeltel, @ahinzmann, @gpetruc, @mariadalfonso, @pvmulder this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@mmusich
Copy link
Contributor

mmusich commented May 12, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@mulhearn
Copy link
Contributor

+1

@mulhearn
Copy link
Contributor

Many many many thanks for this. This was a real nightmare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet