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

Fastsim newrechits rebase sep4 #11121

Merged
merged 19 commits into from Sep 9, 2015
Merged

Fastsim newrechits rebase sep4 #11121

merged 19 commits into from Sep 9, 2015

Conversation

lveldere
Copy link
Contributor

@lveldere lveldere commented Sep 4, 2015

rebase of #11078

  • fastsim tracker rechit classes are replaced with
    • a fastsim base class FastTrackerRecHit
    • a fastsim class for single rechits FastSingleTrackerRecHit
      (accomodates the fastsim equivalents of SiPixelRecHit, SiStripRecHit2D and SiStripRecHit1D)
    • a fastsim equivalent of SiStripMatchedRecHit2D, FastMatchedTrackerRecHit
    • a fastsim equivalent of ProjectedSiStripRecHit2D, FastProjectedTrackerRecHit
  • trackerHitRTTI::RTTI is extended to accommodate the new rechit containers
  • a new implementation of FastTrackerRecHit::sharesInput
    ( will receive further study once merged)
  • rechit production is further modularised
    • actual rechit production by the old SiTrackerGaussianSmearingRecHitConverter
    • hit matching by new FastTrackerRecHitMatcher
    • predefined combinations of hits by new FastTrackerRecHitCombiner
  • SiTrackerGaussianSmearingRecHitConverter produces now
    OwnVector
    vector<\Ref > with one entry per SimHit to find for each SimHit the corresponding RecHit
  • predefined combinations of hits are now in a new, simple format:
    vector<vector<Ref > >
  • model for hit splitting has been improved and moved to a separate class FastTrackerRecHitSplitter
  • model for masking of hits in iterative tracking improved
    (see HitMaskHelper and FastTrackerRecHitMaskProducer)
  • GlobalPixelTracking is not fully migrated yet (it produces zero result):
    it was not properly configured to begin with,
    and we need to study the fullsim equivalent before fixing it
  • various code files were updated for the new rechit classes and the new rechit combination format
  • y position of strip hits is set to 0 (before it was the simhit y-position)
  • unit tests: a few fastsim events are generated on the fly, such to circumvent the backward incompatibility introduced by this pr. Unit tests to be restored when new relvals are available.

@lveldere
Copy link
Contributor Author

lveldere commented Sep 4, 2015

please test

@cmsbuild cmsbuild added this to the Next CMSSW_7_6_X milestone Sep 4, 2015
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2015

A new Pull Request was created by @lveldere for CMSSW_7_6_X.

Fastsim newrechits rebase sep4

It involves the following packages:

CommonTools/RecoAlgos
DataFormats/TrackerRecHit2D
FastSimulation/Configuration
FastSimulation/Muons
FastSimulation/Tracking
FastSimulation/TrackingRecHitProducer
PhysicsTools/PatAlgos
RecoJets/JetAssociationProducers
RecoMET/METProducers
RecoParticleFlow/PFSimProducer
RecoTracker/TransientTrackingRecHit
SLHCUpgradeSimulations/Geometry
SimGeneral/MixingModule
SimTracker/TrackerHitAssociation

@civanch, @Dr15Jones, @lveldere, @cvuosalo, @ianna, @mdhildreth, @monttj, @cmsbuild, @ssekmen, @slava77, @vadler can you please review it and eventually sign? Thanks.
@ghellwig, @TaiSakuma, @abbiendi, @rappoccio, @Martin-Grunewald, @istaslis, @threus, @mmarionncern, @imarches, @makortel, @acaudron, @jhgoh, @lgray, @jdolen, @ferencek, @cerati, @schoef, @wmtan, @GiacomoSguazzoni, @rovere, @VinInn, @nhanvtran, @wmtford, @mschrode, @dgulhan, @ahinzmann, @gpetruc, @matt-komm, @mariadalfonso, @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.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2015

@lveldere
Copy link
Contributor Author

lveldere commented Sep 4, 2015

+1

fullsim comparisons look okay
fastsim changes are expected

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @monttj, @slava77, @vadler
It would be extremely helpful if you could treat this with some priority.
I only have a few working days left before holidays...

@civanch
Copy link
Contributor

civanch commented Sep 7, 2015

+1

int32_t icomb = fastTrackingHelper::getRecHitCombinationIndex(seed);
if(icomb < 0 || unsigned(icomb) >= recHitCombinations->size()){
edm::LogError("TrackCandidateProducer") << " found seed with recHitCombination out or range: " << icomb << std::endl;
exit(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be removed and replaced with an exception

@slava77
Copy link
Contributor

slava77 commented Sep 9, 2015

+1

  • code changes are mostly OK
    • *exit(1) system calls should still be removed * (this can be done in a separate PR)
    • fullsim/data reco is not affected
  • jenkins test pass and comparisons with baseline show differences only in the fastsim workflows
  • I scrolled through a large fraction of comparison plots in 135.4 (ZEE fastsim) and the distributions changed mainly in the tracking algorithm side and less so in the general track or other variables properties
    wf135p4_gentrk_algo
    wf135p4_gentrk_hits_v_eta
  • logfiles still suggest there is a problem in the TrajectorySeedProducer:pixelTripletSeeds:
    this message changed from something that appears frequently and is filling up the log file
Either region producer, or the seed creator cfg is not available.

to something that shows up less frequently

 RegionFactory is not initialised properly, please check your configuration. Producing empty seed collection

@lveldere
Copy link
Contributor Author

lveldere commented Sep 9, 2015

@slava77
Thanks for the comments!
I will get rid of the exits as soon as this pr gets merged.
About the log files: this is a known issue in the HLT paths.
I have a branch with a fix, and I will make a pr when this one gets merged.

davidlange6 added a commit that referenced this pull request Sep 9, 2015
@davidlange6 davidlange6 merged commit 070bdb6 into cms-sw:CMSSW_7_6_X Sep 9, 2015
@Dr15Jones
Copy link
Contributor

@davidlange6 The removal of the class SiTrackerGSMatchedRecHit2D is causing the IB RelVals to fail with the message

----- Begin Fatal Exception 11-Sep-2015 10:25:21 CEST-----------------------
An exception of category 'FileReadError' occurred while
   [0] Processing run: 1 lumi: 1 event: 1
   [1] Running path 'datamixing_step'
   [2] Calling event method for module DataMixingModule/'mixData'
   [3] Reading branch TrackingRecHitsOwned_mix_generalTracks_DIGI2RAW.
   Additional Info:
      [a] Fatal Root Error: @SUB=TBufferFile::ReadObject
trying to read an emulated class (SiTrackerGSMatchedRecHit2D) to store in a compiled pointer (TrackingRecHit)

----- End Fatal Exception -------------------------------------------------

see
https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc6_amd64_gcc493/CMSSW_7_6_X_2015-09-10-2300/pyRelValMatrixLogs/run/250400.0_ZEE_13+FS_ZEE_13_PRMXUP15_PU25+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+FS_ZEE_13_PRMXUP15_PU25+HARVESTUP15FS+MINIAODMCUP15FS.log

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

6 participants