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

MeasurementTracker redesign #915

Merged
merged 4 commits into from Oct 11, 2013

Conversation

gpetruc
Copy link
Contributor

@gpetruc gpetruc commented Sep 25, 2013

MeasurementTracker Redesign, as presented in the RECO meeting on 2013-1019
Rebased to today's IB, and squashed to a single commit.
https://indico.cern.ch/getFile.py/access?contribId=8&sessionId=0&resId=1&materialId=slides&confId=265609
Note: HLT changes are needed, and HLT people are aware of it.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gpetruc for CMSSW_7_0_X.

MeasurementTracker redesign

It involves the following packages:

RecoTracker/TkTrackingRegions
RecoEgamma/EgammaElectronProducers
RecoTracker/ConversionSeedGenerators
RecoMuon/TrackerSeedGenerator
RecoTracker/NuclearSeedGenerator
RecoEgamma/EgammaPhotonAlgos
RecoMuon/L3TrackFinder
CalibTracker/SiStripHitEfficiency
RecoTracker/CkfPattern
Configuration/StandardSequences
RecoTracker/SpecialSeedGenerators
TrackingTools/MeasurementDet
RecoEgamma/EgammaPhotonProducers
RecoEgamma/EgammaElectronAlgos
RecoTracker/Configuration
RecoTracker/DebugTools
FastSimulation/Tracking
RecoTracker/MeasurementDet
RecoTracker/TrackProducer
FastSimulation/Muons
RecoTracker/IterativeTracking

@smuzaffar, @thspeer, @vlimant, @demattia, @franzoni, @nclopezo, @rcastello, @giamman, @slava77, @fabiocos can you please review it and eventually sign? Thanks.
@ghellwig, @cerati, @GiacomoSguazzoni, @rovere 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.
@ktf you are the release manager for this.

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2013

@gpetruc
Giovanni, "todays IB" is it CMSSW_7_0_X_2013-09-24-1400
or CMSSW_7_0_X_2013-09-25-0200 ?

@gpetruc
Copy link
Contributor Author

gpetruc commented Sep 25, 2013

CMSSW_7_0_X_2013-09-25-0200

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2013

From your commit 44af4d9 parent
70c7f17, it is actually CMSSW_7_0_X_2013-09-25-1400
I wonder how that happened

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2013

Also, it doesn't merge cleanly in CMSSW_7_0_X_2013-09-25-0200

@gpetruc
Copy link
Contributor Author

gpetruc commented Sep 25, 2013

I pulled CMSSW_7_0_X before

On Wed, Sep 25, 2013 at 2:20 PM, slava77 notifications@github.com wrote:

From your commit 44af4d944af4d965878877c4e839083f0983f83b4e0d564parent
70c7f1770c7f17,
it is actually CMSSW_7_0_X_2013-09-25-1400
I wonder how that happened


Reply to this email directly or view it on GitHubhttps://github.com//pull/915#issuecomment-25081394
.

@giamman
Copy link
Contributor

giamman commented Sep 25, 2013

+1

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2013

OK, that's probably the reason.
There seem to be enough diffs in Configuration/PyReleaseValidation/python/relval_steps.py between -0200 and -1400 to expect that the incremental diffs have to be done with respect to -1400
If nothing changes, I'll wait for -1400 IB to show up

@nclopezo
Copy link
Contributor

-1
I ran the jenkins tests and I got the following errors in the RelVals:
in workflows 4.53, 5.1, 401.0, 1306, 8.0 and 25.0:

globaltag = PRE_P62_V8::All
271 DQMStore::DQMStore 
25-Sep-2013 14:10:31 CEST  Initiating request to open file root://eoscms//eos/cms/store/data/Run2012B/SinglePhoton/RAW/v1/000/194/533/1084D9DA-AEA2-E111-B3AB-001D09F24DA8.root?svcClass=default
25-Sep-2013 14:10:31 CEST  Successfully opened file root://eoscms//eos/cms/store/data/Run2012B/SinglePhoton/RAW/v1/000/194/533/1084D9DA-AEA2-E111-B3AB-001D09F24DA8.root?svcClass=default
25-Sep-2013 14:10:38 CEST  Closed file root://eoscms//eos/cms/store/data/Run2012B/SinglePhoton/RAW/v1/000/194/533/1084D9DA-AEA2-E111-B3AB-001D09F24DA8.root?svcClass=default
----- Begin Fatal Exception 25-Sep-2013 14:10:38 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=MeasurementTrackerSiStripRefGetterProducer label='hltSiStripClusters'
Exception Message:
MissingParameter: Parameter 'measurementTracker' not found.
----- End Fatal Exception -------------------------------------------------

and 4.55, 100.0:

globaltag = PRE_62_V8::All
271 DQMStore::DQMStore 
25-Sep-2013 14:10:38 CEST  Initiating request to open file root://eoscms//eos/cms/store/data/Run2011A/Cosmics/RAW/v1/000/160/960/049F6443-8E53-E011-A943-003048F117EA.root?svcClass=default
25-Sep-2013 14:10:38 CEST  Successfully opened file root://eoscms//eos/cms/store/data/Run2011A/Cosmics/RAW/v1/000/160/960/049F6443-8E53-E011-A943-003048F117EA.root?svcClass=default
Begin processing the 1st record. Run 160960, Event 10001082, LumiSection 277 at 25-Sep-2013 14:11:18.969 CEST
----- Begin Fatal Exception 25-Sep-2013 14:11:21 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing run: 160960 lumi: 277 event: 10001082
   [1] Running path 'reconstruction_step'
   [2] Calling event method for module CkfTrackCandidateMaker/'ckfTrackCandidatesP5'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: MeasurementTrackerEvent
Looking for module label: MeasurementTrackerEvent
Looking for productInstanceName: 
   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.
----- End Fatal Exception -------------------------------------------------
25-Sep-2013 14:11:22 CEST  Closed file root://eoscms//eos/cms/store/data/Run2011A/Cosmics/RAW/v1/000/160/960/049F6443-8E53-E011-A943-003048F117EA.root?svcClass=default

You can see the logs here:
https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc481/645/consoleFull
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/645

@gpetruc
Copy link
Contributor Author

gpetruc commented Sep 25, 2013

Hi David,

4.53, 5.1, 401.0, 1306, 8.0 and 25.0 --> yes this is expected: configuration changes are needed from the HLT side.

4.55, 100.0 --> the MeasurementTrackerEvent producer has to be inserted also in the Cosmic reco sequence, as done for the collision one as in gpetruc@44af4d9#diff-bc1d3eddbea8b75d590d62e72fee751e
@slava77 , @thspeer : I can add it to the trackerCosmics sequence in between trackerlocalreco and tracksP5, in a separate commit. Do you want it on this branch or on a second branch to keep this immutable?

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2013

Hi Giovanni,

I think it's safe to update this branch so that we get fixes in one PR instead of rolling from a more broken one to a less broken one

@cmsbuild
Copy link
Contributor

Pull request #915 was updated. @smuzaffar, @thspeer, @vlimant, @demattia, @franzoni, @nclopezo, @rcastello, @giamman, @slava77, @fabiocos can you please check and sign again.

@gpetruc
Copy link
Contributor Author

gpetruc commented Sep 25, 2013

Ok, I've fixed the cosmic reco (workflow 1000.0) in commit gpetruc@f8acb34

I assume that David meant workflow 1000.0 instead of 100.0 (which doesn't seem to exist) and that worflow 4.55 has the same HLT error as the others (it's a re-HLT workflow, not a cosmic workflow)

@giamman
Copy link
Contributor

giamman commented Sep 25, 2013

+1
again.

@slava77
Copy link
Contributor

slava77 commented Sep 26, 2013

@slava77 working on it

@slava77
Copy link
Contributor

slava77 commented Sep 26, 2013

@Martin-Grunewald
Hi Martin,
what should be the steps now to get the changes needed for HLT?

@Martin-Grunewald
Copy link
Contributor

Hi,

I am already working on it!

Best regards

Martin

On Thu, 26 Sep 2013, slava77 wrote:

@Martin-Grunewald
Hi Martin,
what should be the steps now to get the changes needed for HLT?


Reply to this email directly or view it on GitHub:
#915 (comment)

Martin

Martin W. Gruenewald e-mail: Martin.Grunewald@cern.ch
http://cern.ch/Martin.Grunewald

@gpetruc
Copy link
Contributor Author

gpetruc commented Sep 26, 2013

I have added to the pull request one commit with some changes in the cfi that were requested by @Martin-Grunewald for HLT configuration parsing.
Offline tracking is not affected by this at all.

@cmsbuild
Copy link
Contributor

Pull request #915 was updated. @smuzaffar, @thspeer, @vlimant, @demattia, @franzoni, @nclopezo, @rcastello, @giamman, @slava77, @fabiocos can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #915 was updated. @smuzaffar, @thspeer, @vlimant, @demattia, @franzoni, @nclopezo, @rcastello, @giamman, @slava77, @fabiocos can you please check and sign again.

@giamman
Copy link
Contributor

giamman commented Sep 27, 2013

+1

@giamman
Copy link
Contributor

giamman commented Oct 9, 2013

Why wasn't this included in pre5? Sorry if I missed some discussion about that.

@ktf
Copy link
Contributor

ktf commented Oct 9, 2013

As far as I know there is more work on the HLT side to be done before this can enter CMSSW_7_0_X.

@ktf
Copy link
Contributor

ktf commented Oct 9, 2013

Would it help if we set up a special IB for this?

@ktf ktf merged commit 51fa268 into cms-sw:CMSSW_7_0_X Oct 11, 2013
@gpetruc gpetruc deleted the MeasurementTracker-redesign-v1 branch September 30, 2015 21:44
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

7 participants