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

Second part of the migration to era for phase2 tracking #15417

Merged

Conversation

ebrondol
Copy link
Contributor

This PR is a continuation of the PR #15183 .
The entire customise_Reco method as been migrated to era for both tilted and flat geometry.
Mostly of the migration has been done following the Phase1 migration.
No difference is expected with the baseline, here the comparison for flat and tilted using the MTV. No difference is also expected for phase0/1 iterative tracking.

Thanks to @makortel for the precious help (again and again!).
Informing @VinInn @rovere @delaere @boudoul @kpedro88

moving inactivePixelDetectorLabels to era

Trying to move siPixelCluster.src but gives changes..

Moving ClusterShapeHitFilterESProducer.PixelShapeFile to era

Moving MeasurementTrackerEvent and preDuplicateMergingDisplaced to era

moving phase2ITPixelClusters to eras and first attempt for the EventContent

moving to era recoFromSimDigis_cff.py and globalreco_tracking
moving regional cosmics

fixing MissCalibrate
moving phase2TrackerClusterizer_cfi from Sim folder to Reco

fix PreSplitting part
cleaning

removing recoFromSimDigis_cff

fix pixelTracks

cleaning up
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ebrondol for CMSSW_8_1_X.

It involves the following packages:

Configuration/StandardSequences
RecoLocalTracker/Configuration
RecoLocalTracker/Phase2ITPixelClusterizer
RecoLocalTracker/SiPixelClusterizer
RecoLocalTracker/SiPixelRecHits
RecoLocalTracker/SiStripClusterizer
RecoLocalTracker/SiStripZeroSuppression
RecoMuon/Configuration
RecoPixelVertexing/Configuration
RecoPixelVertexing/PixelLowPtUtilities
RecoPixelVertexing/PixelTrackFitting
RecoTracker/IterativeTracking
RecoTracker/MeasurementDet
RecoTracker/SpecialSeedGenerators
SLHCUpgradeSimulations/Configuration
SLHCUpgradeSimulations/Geometry
SimTracker/SiPhase2Digitizer

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @franzoni, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @forthommel, @abbiendi, @Martin-Grunewald, @threus, @battibass, @makortel, @sdevissc, @jhgoh, @jlagram, @HuguesBrun, @OlivierBondu, @trocino, @rociovilar, @yduhm, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @mschrode, @dgulhan, @nickmccoll, @gbenelli, @dkotlins, @gpetruc, @bachtis this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@kpedro88
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 10, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14462/console

@cmsbuild
Copy link
Contributor

@ebrondol
Copy link
Contributor Author

@kpedro88 do you think this will have conflict with #15393?
About possible conflict with #15075, all the changes in the customization file in that PR are done in customise_DigiTkOnly, so I suppose there is no conflict also with that. Already when this PR has been proposed by @sviret, I was suggesting to put the modifications already in era but this has not been done and I am not sure how that part is going to be migrated (and when).

@kpedro88
Copy link
Contributor

@ebrondol there are no modified files in common between this and #15393, so there shouldn't be any conflict there.

As for #15075, there are a lot of edits in the phase2Tk*.py files. Sometimes the diff is too complicated for git to figure out there isn't an actual conflict, in which case a rebase would still be required.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 12, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14513/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

cvuosalo commented Aug 16, 2016

Jenkins tests and extended tests of workflows 10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D1 and 10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D2 with 30 events each against baseline CMSSW_8_1_X_2016-08-11-2300 show no significant differences in monitored quantities. The PR does cause a very slight change in the number of messages logged.

@ebrondol
Copy link
Contributor Author

@cvuosalo thanks for the report, can you please copy one of the error message?

@cvuosalo
Copy link
Contributor

The extended tests described above show a change in the modules running. The following modules have been added:

CastorTowerReco
ak5CastorJets
ak7CastorJets
siPixelRecHitsPreSplitting
PixelLayerTripletsPreSplitting
siPixelClusterShapeCachePreSplitting
castorreco
ak7CastorJetID
ak5CastorJetID
siPixelClustersPreSplitting

One module was removed: PixelLayerTriplets

@cvuosalo
Copy link
Contributor

@ebrondol: I can't find any difference in the error messages. It could be that the additional modules change the timing and slightly change the diagnostic logs.

@cvuosalo
Copy link
Contributor

@ebrondol: You say in the PR description, "No difference is expected with the baseline." However, this PR adds and removes running modules. How can you change which modules are running and yet produce no differences?

@ebrondol
Copy link
Contributor Author

@cvuosalo Ah, ok, so I think I misunderstood your comment "The PR does cause a very slight change in the number of error messages logged.". Is it referred to other Log messages and not LogError?

About the second question, on the tracking point of view, the modules that have been changed are, for example: PixelLayerTriplets -> PixelLayerTripletsPreSplitting. If they are set in the correct way, those modifications should not produce any changes in the reco. @makortel can confirm.
About the part for tha calorimeter, @kpedro88 asked me to put these modules in, run the usual workflows and check any problems. Since no problems has been found, I think that this modules are not actually used in the overall reco for phase2, but maybe he can comment a bit more.

@cvuosalo
Copy link
Contributor

@ebrondol: The only evidence for changes in logging are in the validation plots. I don't think they are significant.
all_testpr15417vsorig_ttbar13tev2023d1wf10424p0c_edmerrorsummaryentrys_logerrorharvester__reco_objat_size
all_testpr15417vsorig_ttbar13tev2023d1wf10424p0c_edmerrorsummaryentrys_logerrorharvester__reco_obj_module_size

@cvuosalo
Copy link
Contributor

+1

For #15417 7ce925d

Completing era migration for Phase 2 tracking. There should be no change in monitored quantities.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-08-12-1100 show no significant differences, as expected. Extended tests as described above also show no significant differences. Changes in running modules as discussed above are desired and correct.

@civanch
Copy link
Contributor

civanch commented Aug 19, 2016

+1

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