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: new rechits containers #11078

Closed
wants to merge 20 commits into from
Closed

FastSim: new rechits containers #11078

wants to merge 20 commits into from

Conversation

lveldere
Copy link
Contributor

@lveldere lveldere commented Sep 2, 2015

rebase of #10987

  • 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 2, 2015

please test

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

cmsbuild commented Sep 2, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

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

FastSim: new rechits containers

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 2, 2015

-1
Tested at: 6c9fbe9
When I ran the RelVals I found an error in the following worklfows:
1001.0 step1

DAS Error

1003.0 step1

DAS Error

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

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
ac66418
28ce045
72ecfd0
6acf0f1
5c6a928
97c4b3d
b58c545
ffa4c20
0f7191b
9699681
4720ec2
78e70d3
bba7104
5ec3e31
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11078/7809/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11078/7809/git-merge-result

@lveldere
Copy link
Contributor Author

lveldere commented Sep 2, 2015

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 2, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 3, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 3, 2015

@lveldere
Copy link
Contributor Author

lveldere commented Sep 3, 2015

@slava77 @makortel @rovere @VinInn

The fastsim comparisons behave like expected,
except for distributions related to tracking distributions related to dzpvcut. [1]

The strange thing:
the fullsim comparisons for 25202 ( ttbar + 25ns PU) look ok,
while the fullsim comparisons for 50202 (ttbar + 50ns PU) show
differences related to tracking distributions related to dzpvcut, just like FullSim. [2]

I'll stare a bit further to my code,
but if you have ideas about what could be going on
(perhaps interference with other commits included in the tests, or some change in the global tags)

Many thanks!

[1]
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_7_6_X_2015-09-02-1100+11078/9798/5.1_TTbar+TTbarFS+HARVESTFS/Tracking__Track_cutsRecoHp_trackingParticleRecoAsssociation_num_assoc(recoToSim)_dzpvcut_pt.png

[2]
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_7_6_X_2015-09-02-1100+11078/9798/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/Tracking__Track_general_trackingParticleRecoAsssociation_num_assoc(recoToSim)_dzpvcut_pt.png

@makortel
Copy link
Contributor

makortel commented Sep 3, 2015

@lveldere The dzpvcut histograms seem to be very sensitive to any changes in z positions of vertices and PCA's of tracks (and since they are cumulative, small differences accumulate).

50202 shows quite a bit of changes also elsewhere (alternative-comparisons has 8027 pages), e.g. in TrackingParticles
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_7_6_X_2015-09-02-1100+11078/9798/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/Tracking_TrackingMCTruth_TrackingParticle.html
OTOH primary TrackingVertices do not show any difference, but the vertex validation does show differences (starting from page 7846 in alternative-comparisons, I wonder why the RelMon does not flag these as some of the plots have sizable differences).

Next I tried to look for plots in OfflinePV/offlinePrimaryVertices, but saw none in either RelMon or alternative-comparisons, so actually now I'm a bit confused. If the primary TrackingVertices do not change, and the offlinePrimaryVertices do not change, why the vertex validation shows differences?

Ok, looked the root files myself, and I see differences in both OfflinePV/offlinePrimaryVertices and Vertexing/PrimaryVertexV/Gen*, so differences in vertex validation and dzpvcut are expected. (and now I wonder even more why these differences are hidden by the comparison tools...)

Is the SIM redone (with different random number sequence) in this wf?

In 5.1, the dzpvcut histograms look reasonable to me, given that there are small differences in tracks.

@lveldere
Copy link
Contributor Author

lveldere commented Sep 3, 2015

Hi Matti

I've also been looking into DQM files.
Not sure what this explains, but going
from CMSSW_7_6_X_2015-09-01-1100
to CMSSW_7_6_X_2015-09-02-1100
I observe similar changes for 25202 (= no changes) and 50202 (changes)

Lukas

On Thu, Sep 3, 2015 at 10:28 AM, Matti Kortelainen
notifications@github.com wrote:

@lveldere The dzpvcut histograms seem to be very sensitive to any changes in
z positions of vertices and PCA's of tracks (and since they are cumulative,
small differences accumulate).

50202 shows quite a bit of changes also elsewhere (alternative-comparisons
has 8027 pages), e.g. in TrackingParticles
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_7_6_X_2015-09-02-1100+11078/9798/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/Tracking_TrackingMCTruth_TrackingParticle.html
OTOH primary TrackingVertices do not show any difference, but the vertex
validation does show differences (starting from page 7846 in
alternative-comparisons, I wonder why the RelMon does not flag these as some
of the plots have sizable differences).

Next I tried to look for plots in OfflinePV/offlinePrimaryVertices, but saw
none in either RelMon or alternative-comparisons, so actually now I'm a bit
confused. If the primary TrackingVertices do not change, and the
offlinePrimaryVertices do not change, why the vertex validation shows
differences?

Ok, looked the root files myself, and I see differences in both
OfflinePV/offlinePrimaryVertices and Vertexing/PrimaryVertexV/Gen*, so
differences in vertex validation and dzpvcut are expected. (and now I wonder
even more why these differences are hidden by the comparison tools...)

Is the SIM redone (with different random number sequence) in this wf?

In 5.1, the dzpvcut histograms look reasonable to me, given that there are
small differences in tracks.


Reply to this email directly or view it on GitHub.

@slava77
Copy link
Contributor

slava77 commented Sep 4, 2015

@lveldere
Hi Lukas,
is this one superseded by #11121 ?
If so, please close this PR.

@lveldere
Copy link
Contributor Author

lveldere commented Sep 4, 2015

new pr: #11121

@lveldere lveldere closed this Sep 4, 2015
@lveldere
Copy link
Contributor Author

lveldere commented Sep 4, 2015

Wanted to keep it open until seeing the comparisons of #11121,
Closed now.
Thanks
Lukas

On Fri, Sep 4, 2015 at 5:24 PM, Slava Krutelyov notifications@github.com
wrote:

@lveldere https://github.com/lveldere
Hi Lukas,
is this one superseded by #11121
#11121 ?
If so, please close this PR.


Reply to this email directly or view it on GitHub
#11078 (comment).

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

4 participants