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

Migrate the temporary 2017 DQM+validation customizations to eras #13987

Merged
merged 1 commit into from Apr 23, 2016

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Apr 7, 2016

Title pretty much says it all. All era-customizations are marked such that they can be found with git grep "phase1Pixel.*FIXME".

These are the last pieces of phase1TkCustoms not yet controlled by eras. After this PR, I'll start cleaning up the customize.

Tested in CMSSW_8_1_X_2016-04-07-1100, no changes expected in standard workflows.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2016

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

It involves the following packages:

DQM/Physics
DQM/SiStripMonitorClient
DQMOffline/Configuration
DQMOffline/JetMET
DQMOffline/Muon
HLTriggerOffline/Common
SLHCUpgradeSimulations/Configuration
Validation/Configuration
Validation/RecoTau
Validation/RecoTrack

@civanch, @cvuosalo, @mdhildreth, @cmsbuild, @deguio, @slava77, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @abbiendi, @rappoccio, @Martin-Grunewald, @threus, @venturia, @mmarionncern, @battibass, @ahinzmann, @jhgoh, @jdolen, @cerati, @trocino, @schoef, @rociovilar, @barvic, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @nhanvtran, @wmtford, @dgulhan, @istaslis, @mariadalfonso this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@makortel
Copy link
Contributor Author

makortel commented Apr 7, 2016

@schneiml You might be interested to see where the switch to turn off/on the pixel (phase1) DQM gets moved to.

@civanch
Copy link
Contributor

civanch commented Apr 8, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2016

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

@@ -31,6 +32,9 @@
es_dqm_source_offline *
castorSources *
HcalDQMOfflineSequence )
eras.phase1Pixel.toReplaceWith(DQMOfflinePreDPG, DQMOfflinePreDPG.copyAndExclude([ # FIXME
siPixelOfflineDQM_source, # Pixel DQM needs to be updated for phase1
Copy link
Contributor

Choose a reason for hiding this comment

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

@makortel Do I uderstand correctly that the new Pixel DQM should/could be plugged in here?

I changed https://github.com/schneiml/cmssw/blob/phase1pixeldqm-histoman-refactoring/DQM/SiPixelCommon/python/SiPixelOfflineDQM_source_cff.py#L133 to introduce eras, which seems to be another alternative to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@schneiml Taking a look on your branch, I would say that

  • this is not a bad place to take out pixel DQM (it could be done also in SiPixelOfflineDQM_source_cff but this is more "blind" approach)
  • replacing phase0 DQM with phase1 DQM is best done in SiPixelOfflineDQM_source_cff as you did

Basically, if/after this PR gets integrated, for enabling the pixel DQM sequence you need to remove these three lines (in addition to the phase0->phase1 replacement you already have).

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2016

@civanch
Copy link
Contributor

civanch commented Apr 11, 2016

+1

@makortel
Copy link
Contributor Author

Ping

@cvuosalo
Copy link
Contributor

+1

For #13987 ce890c4

Migrating 2017 DQM+validation customizations to eras. There should be no change in standard, non-2017 workflows.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-04-07-2300 show no significant differences, as expected.

@makortel
Copy link
Contributor Author

There should be no change in standard, non-2017 workflows.

To be clear, there should be no changes in 2017 workflows either.

@schneiml
Copy link
Contributor

Any updates on this one? I'd iike to see this merged as well...

@makortel
Copy link
Contributor Author

@deguio, @vanbesien Could you please review and sign? Thanks. (@deguio, this is exactly the same commit you looked already before I opened the PR)

@deguio
Copy link
Contributor

deguio commented Apr 22, 2016

+1
right, we already discussed the details. sorry for being late.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_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

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