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

CTPPS: DQM configuration fix #18718

Merged
merged 4 commits into from May 28, 2017
Merged

Conversation

jan-kaspar
Copy link
Contributor

Several fixes for CTPPS DQM configuration. Important for commissioning/calibration at the LHC restart.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jan-kaspar for master.

It involves the following packages:

DQM/CTPPS
DQM/Integration

@cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@threus, @batinkov, @thomreis this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@forthommel
Copy link
Contributor

Hello @slava77, @perrotta, all,
Could somebody please issue the 'please test' command for this PR?

@perrotta
Copy link
Contributor

@forthommel : I can do, but this is not a reco PR.
You should interact with DQM for it

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19820/console Started: 2017/05/15 10:21

@forthommel
Copy link
Contributor

Many thanks, @perrotta!
Sorry for bugging you with this then, old habits are hard to die...!

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18718/19820/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3389 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1830764
  • DQMHistoTests: Total failures: 48242
  • DQMHistoTests: Total nulls: 21
  • DQMHistoTests: Total successes: 1782321
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@forthommel
Copy link
Contributor

@dmitrijus, @vanbesien, can we help you somehow for your review?

@threus
Copy link
Contributor

threus commented May 17, 2017

@jan-kaspar @forthommel could you please make a back port PR for 91X asap? thanks. Tomas (DQM). Broen and Dmitrijus will be available Friday the soonest.

@cmsbuild
Copy link
Contributor

Pull request #18718 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again.

@jan-kaspar
Copy link
Contributor Author

07882d1 also resynchronises this PR with #18780.

@dmitrijus
Copy link
Contributor

I've added a comment, which has to be addressed.

@forthommel
Copy link
Contributor

Hi @dmitrijus
Which comment are you referring to?

@@ -12,15 +12,14 @@
# for testing in lxplus
process.load("DQM.Integration.config.fileinputsource_cfi")
process.source.fileNames = cms.untracked.vstring(
'file:/afs/cern.ch/user/j/jkaspar/public/run273062_ls0001-2_stream.root',
#'root://eostotem.cern.ch//eos/totem/user/j/jkaspar/04C8034A-9626-E611-9B6E-02163E011F93.root'
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work online. You must use "DQM.Integration.config.inputsource_cfi", the "fileinputsource_cfi" is for testing/development only!

Also, for testing, you can also use (with inputsource_cfi): cmsRun <my_online_dqm_client> inputFiles="a.root,b.root"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Dmitrijus,

the lines you talk about would not be executed since the test variable is set to False:
https://github.com/CTPPS/cmssw/blob/07882d194c34f8cf39ea91471c3f9ca176af35bc/DQM/Integration/python/clients/ctpps_dqm_sourceclient-live_cfg.py#L5
Nevertheless, if you prefer, I can remove the "test" block completely - let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed the if block, sorry :(

@dmitrijus
Copy link
Contributor

I forgot to press "submit" button, you should see it now.

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 26, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20141/console Started: 2017/05/26 16:32

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18718/20141/summary.html

Comparison Summary:

  • You potentially added 210 lines to the logs
  • Reco comparison results: 1693 differences found in the comparisons
  • DQMHistoTests: Total files compared: 24
  • DQMHistoTests: Total histograms compared: 1833612
  • DQMHistoTests: Total failures: 58099
  • DQMHistoTests: Total nulls: 35
  • DQMHistoTests: Total successes: 1775298
  • DQMHistoTests: Total skipped: 180
  • DQMHistoTests: Total Missing objects: 0
  • Checked 98 log files, 14 edm output root files, 24 DQM output files

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 01ccdc1 into cms-sw:master May 28, 2017
@jan-kaspar jan-kaspar deleted the ctpps_dqm_config_fix branch June 23, 2017 18:23
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