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

Event based track and muon to simulation associators #7175

Merged
merged 15 commits into from Jan 22, 2015

Conversation

Dr15Jones
Copy link
Contributor

Replaced TrackAssociatorBase class with three classes which are obtained from the Event. This allows
modules to properly register their 'consumes'.
This change should give identical results to the previous implementation.

Previously, TrackAssociatorByChi2 would also match a generated particle
to a track. In order to use it code would get it from the EventSetup
via its base class and then cast it to its actual type. By creating
a seperate TrackGenAssociatorBase and TrackGenAssociatorByChi2 it
avoids the use of casting.

This is a step towards fixing the consumes problem with TrackAssociators.
Previously, code was getting a TrackAssociatorBase from the EventSetup
and casting it to a MuonAssociatorByHits in order to do muon association.
This change creates a separate MuonToSimAssociatorBase which provides
the correct interface. This new object can be obtained from the EventSetup
and is created by MuonAssociatorESProducer. Common code was moved to
MuonAssociatorByHitsHelper.
This change is needed as a step towards fixing the consumes problem
with TrackAssociatorBase.
Made the routines used by MuonAssociatorByHitsHelper const.
The MuonAssociatorByHitsHelper was only using a small portion of
MuonTruth and no other user of MuonTruth used that portion. Splitting
the functionality to its own class helps reduce dependencies and avoids
unnecessary time and memory.
Classes which use MuonAssociatorByHitsHelper are now responsible for
passing it the Event and EventSetup based resources it needs.
The various Reco to Sim assocation implementations require each
implementation to use additional information from the Event and/or
EventSetup. Inorder to accommodate that and to get the 'consumes' to
work, the association classes need to be Event data products.

This change provides the event data product for the three types of
associations and the base class for the actual implementations of
the associations. Classes which inherit from the base class are
defined in additional pacakges. This works since the associations are
transient only.
The implementations of the associations are all copies of the
code in SimTrack/TrackAssociation where the Event or EventSetup
data products are held by the product rather than obtained from
the Event or EventSetup each call. These implement the interface
defined in SimDataFormats/Associations.
The implementations of the associations are all copies of the
EventSetup based product code  where the Event or EventSetup
data products are held by the product rather than obtained from
the Event or EventSetup   each call. This implement the interface
defined in SimDataFormats/Associations.
…rBase

The TrackAssociatorBase lives in the EventSetup but gets data from both the
edm::Event and edm::EventSetup. This is not allowed by CMSSW rules. Therefore
TrackAssociatorBase is replaced by reco::TrackToTrackingParticleAssociator which
is obtained via the edm::Event.
This solves the consumes problem associated with TrackAssociatorBase usage.
TrackGenAssociatorBase is an EventSetup object which uses the edm::Event
to get data. This is a problem for consumes. So instead we nw use the
reco::TrackToGenParticleAssociator which does the same work but is an
edm::Event product which already contains the data it needs from the event.
…atorBase

MuonToSimAssociatorBase is an EventSetup object which uses the edm::Event
to get data. This is a problem for consumes. So instead    we nw use the
reco::MuonToTrackingParticleAssociator which does the same work but  is an
edm::Event product which already contains the data it needs from the event.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_4_X.

Event based track and muon to simulation associators

It involves the following packages:

Alignment/OfflineValidation
CalibTracker/SiStripCommon
Configuration/StandardSequences
DPGAnalysis/SiStripTools
FastSimulation/Validation
MuonAnalysis/MuonAssociators
RecoBTag/SoftLepton
RecoMuon/TrackingTools
RecoTracker/DebugTools
RecoVertex/ConfigurableVertexReco
RecoVertex/KalmanVertexFit
RecoVertex/KinematicFit
SLHCUpgradeSimulations/Geometry
SimDataFormats/Associations
SimMuon/MCTruth
SimTracker/TrackAssociation
SimTracker/TrackAssociatorESProducer
SimTracker/TrackAssociatorProducers
SimTracker/TrackHistory
Validation/RecoEgamma
Validation/RecoHI
Validation/RecoMuon
Validation/RecoTrack
Validation/RecoVertex

The following packages do not have a category, yet:

SimDataFormats/Associations
SimTracker/TrackAssociatorProducers

@civanch, @diguida, @lveldere, @cvuosalo, @ianna, @mdhildreth, @monttj, @cmsbuild, @cerminar, @franzoni, @Dr15Jones, @rcastello, @deguio, @slava77, @danduggan, @vadler, @mmusich, @davidlange6, @ktf, @nclopezo can you please review it and eventually sign? Thanks.
@ghellwig, @abbiendi, @Martin-Grunewald, @tlampen, @threus, @venturia, @frmeier, @battibass, @makortel, @acaudron, @dgulhan, @jlagram, @ferencek, @cerati, @trocino, @rociovilar, @kkrajczar, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @wmtford, @mmusich, @richard-cms, @imarches, @gbenelli, @appeltel, @RylanC24, @gpetruc, @matt-komm, @pvmulder, @bachtis 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.

@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-1
Tested at: c7ef28a
I found an error when building:

Copying tmp/slc6_amd64_gcc491/src/SimMuon/MCTruth/src/SimMuonMCTruth/libSimMuonMCTruth.so to productstore area:
Leaving library rule at SimMuon/MCTruth
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoTrack/src/MTVHistoProducerAlgoFactory.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoTrack/src/MTVHistoProducerAlgoForTracker.cc 
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoB/plugins/recoBSVTagInfoValidationAnalyzer.cc: In constructor 'recoBSVTagInfoValidationAnalyzer::recoBSVTagInfoValidationAnalyzer(const edm::ParameterSet&)':
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoB/plugins/recoBSVTagInfoValidationAnalyzer.cc:95:121: error: no matching function for call to 'VertexClassifierByProxystd::vector, reco::JTATagInfo>, reco::Vertex> > >::VertexClassifierByProxy(const edm::ParameterSet&)'
 recoBSVTagInfoValidationAnalyzer::recoBSVTagInfoValidationAnalyzer(const edm::ParameterSet& config) : classifier_(config)
                                                                                                                         ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoB/plugins/recoBSVTagInfoValidationAnalyzer.cc:95:121: note: candidates are:
In file included from /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/Validation/RecoB/plugins/recoBSVTagInfoValidationAnalyzer.cc:21:0:
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_7_4_X_2015-01-14-1400/src/SimTracker/TrackHistory/interface/VertexClassifierByProxy.h:20:5: note: VertexClassifierByProxy::VertexClassifierByProxy(const edm::ParameterSet&, edm::ConsumesCollector&&) [with Collection = std::vectorreco::TemplatedSecondaryVertexTagInfo, reco::JTATagInfo>, reco::Vertex> >]


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7175/1872/summary.html

@davidlange6
Copy link
Contributor

bypassing the remaining signatures on this one given the lack of complaints (so that it doesn't get out of sync and create more work)

davidlange6 added a commit that referenced this pull request Jan 22, 2015
Event based track and muon to simulation associators
@davidlange6 davidlange6 merged commit cdfc6f9 into cms-sw:CMSSW_7_4_X Jan 22, 2015
@ktf
Copy link
Contributor

ktf commented Jan 23, 2015

Apparently this one breaks one test in the gcc481 build due to some unused variable.

@Dr15Jones can you have a look?

@Dr15Jones
Copy link
Contributor Author

gcc4.8.1 compile problem fixed in #7340

@Dr15Jones Dr15Jones deleted the eventBasedTrackAssociation branch January 23, 2015 16:31
@kkrajczar
Copy link
Contributor

Dear all,

It seems that this PR broke the HI RelVals. The related error message
that we see is "RuntimeError: An entry in sequence tracksValidationFS
has no label". Please find below a description of the problem and a
pointer to a PR with a workaround.

In Thursday's IB (CMSSW_7_4_X_2015-01-22-0200) the following cmsDriver
command correctly produces a python config:

cmsDriver.py step3 --conditions auto:run2_mc_HIon -s
RAW2DIGI,L1Reco,RECO,VALIDATION,DQM -n 2 --eventcontent RECOSIM,DQM
--scenario HeavyIons --datatier GEN-SIM-RECO,DQMIO --customise
SLHCUpgradeSimulations/Configuration/postLS1Customs.customisePostLS1
--magField 38T_PostLS1 --no_exec

However, the same command in Friday's IB (CMSSW_7_4_X_2015-01-23-0200)
crashes with the error message "RuntimeError: An entry in sequence
tracksValidationFS has no label". It seems that the crash is caused by
this PR, as Thursday's build + PR #7175 results in the
above-mentioned error, while Thursday's build itself is ok.

We don't really understand the reason of the error, but we prepared a
workaround PR #7404 for the HI Validation setup, so that the issue does not
shadow further problems with upcoming PRs. We offer our help for testing
in any level, however we really need expertise help in the actual fix.

(@yetkinyilmaz and @BetterWang probably want to follow this)

@makortel
Copy link
Contributor

While investigating something unrelated within the track-TP associations, I noticed that we have still the old ESProduct-based implementations in SimTracker/TrackAssociation (that are, so far, nearly exact copies of these ones). Are the old ones still going to be supported/(used), or could they be cleaned up? I'm just afraid that the current situation will eventually lead to confusion (especially if/when something needs to be changed in them).

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