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

Remove last bits of LargeD0 iterations, and add Phase1 iteration enumerators #12458

Merged
merged 2 commits into from Nov 25, 2015

Conversation

makortel
Copy link
Contributor

This PR removes the last remnants of LargeD0 iterations (they were partly removed already in #3446, and were confirmed to be obsolete https://hypernews.cern.ch/HyperNews/CMS/get/recoTracking/1583/1.html). Then, as a preparatory step, the iterNLargeD0 track algorithm enumerators are reused for upcoming new Phase1 tracking iterations (two of them are not needed now, so they're marked as reserved). They are intentionally not yet used (in Phase1PU70 and PU140 configurations), because I want to keep the current state until we have finalized the validation of Phase1 tracking in 80X against 62X_SLHC.

Tested in 800pre2, no changes expected.

@rovere @VinInn

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_0_X.

It involves the following packages:

DataFormats/TrackReco
RecoTracker/FinalTrackSelectors
RecoTracker/IterativeTracking

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @istaslis, @gpetruc, @cerati, @dgulhan this is something you requested to watch as well.
@Degano you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@slava77
Copy link
Contributor

slava77 commented Nov 17, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

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

DAS Error

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

@slava77
Copy link
Contributor

slava77 commented Nov 18, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@VinInn
Copy link
Contributor

VinInn commented Nov 18, 2015

There are, in principle, two PFlow file to modify...
RecoParticleFlow/PFProducer/src/PFElectronAlgo.cc
and
RecoParticleFlow/PFTracking/src/PFDisplacedVertexFinder.cc

I was doing something similar in
CMSSW_8_0_X...VinInn:SeedRedux800
that I will align to these changes...

We may thing also to "create" a place holder for the duplicateMerger (in place of rs ?)
What is the plan for HighPtQuadruplet: to use InitialStep?

@makortel
Copy link
Contributor Author

@VinInn Yes, eventually those PF files need to be modified (and also elsewhere where the algo enumerators are used explicitly, I haven't checked yet). I didn't include those here because the new enumerators are not yet used anywhere. I was planning to do the migrations in the same PR I change the current Phase1PU70/140 iterations to set the proper algo (which will have to wait a bit as I wrote in the PR description).

I was indeed thinking to use initialStep for the first (quadruplet-seeded) Phase1 iteration.

@VinInn
Copy link
Contributor

VinInn commented Nov 18, 2015

@makortel , ok. let's keep the list of the file involved somewhere....

@cmsbuild
Copy link
Contributor

-1
Tested at: 8e86aba
I found an error when building:

>> Compiling python modules src/CommonTools/RecoUtils/python
>> Compiling python modules src/ElectroWeakAnalysis/ZMuMu/python
>> Pluging of all type refreshed.
>> All python modules compiled
gmake[1]: Leaving directory `/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2015-11-17-1100'
gmake: **\* [There are compilation/build errors. Please see the detail log above.] Error 1


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

@makortel
Copy link
Contributor Author

The error seems to be

/bin/cp: /bin/cp: cannot create regular file `cfipython/slc6_amd64_gcc493/DQM/BeamMonitor/FedRawDataInputSource_cfi.py'cannot create regular file `cfipython/slc6_amd64_gcc493/DQM/BeamMonitor/FedRawDataInputSource_cfi.py': File exists
: File exists
gmake: *** [lib/slc6_amd64_gcc493/AlcaBeamMonitor.edmplugin] Error 1
gmake: *** [lib/slc6_amd64_gcc493/BeamSpotProblemMonitor.edmplugin] Error 1

I can't see how this could be caused by this PR.

@smuzaffar
Copy link
Contributor

assign daq
actual issue is that all the plugins generated from DQM/BeamMonitor [a] are generating the same cfi file. So plugin gmake processes trying to copy this file and fail. Actual problem is in
https://github.com/cms-sw/cmssw/blob/8912b6fabc351091aedf219db64948df22a1ff85/EventFilter/Utilities/src/FedRawDataInputSource.cc#L1287
where this shared library is defining a plugin. Thisplugin definition should move to EventFilter/Utilities/plugins

AlcaBeamMonitor/edm_write_config/FedRawDataInputSource_cfi.py        BeamMonitor/edm_write_config/FedRawDataInputSource_cfi.py             PixelVTXMonitor/edm_write_config/FedRawDataInputSource_cfi.py
AlcaBeamMonitorClient/edm_write_config/FedRawDataInputSource_cfi.py  BeamMonitorBx/edm_write_config/FedRawDataInputSource_cfi.py           TKStatus/edm_write_config/FedRawDataInputSource_cfi.py
BeamConditionsMonitor/edm_write_config/FedRawDataInputSource_cfi.py  BeamSpotProblemMonitor/edm_write_config/FedRawDataInputSource_cfi.py  Vx3DHLTAnalyzer/edm_write_config/FedRawDataInputSource_cfi.py

@VinInn
Copy link
Contributor

VinInn commented Nov 20, 2015

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@VinInn
Copy link
Contributor

VinInn commented Nov 24, 2015

@slava77 , @davidlange6 this is purely technical.
We need it integrated to continue working on this and other iterations
In particular I have to make duplicateMerged an algo by its own, this will produce (apparent) regression.
So better to do it as early as possible...

@slava77
Copy link
Contributor

slava77 commented Nov 24, 2015

+1

for #12458 8e86aba

  • technical changes only to replace unused iteration enums with phase1 and phase2 placeholders
  • jenkins tests pass and comparisons with baseline show no differences as expected

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Nov 25, 2015
Remove last bits of LargeD0 iterations, and add Phase1 iteration enumerators
@cmsbuild cmsbuild merged commit fecf2a9 into cms-sw:CMSSW_8_0_X Nov 25, 2015
@makortel makortel deleted the trackBasePhase1Algos branch October 20, 2016 11:48
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