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

Made simple_stat.h standalone #21412

Merged
merged 1 commit into from
Nov 30, 2017
Merged

Conversation

Teemperor
Copy link
Contributor

This header is lacking header guards and it uses std::max but
doesn't include the algorithm header. This path just adds
the missing guards and the right include to make this header
work with modules.

@Teemperor
Copy link
Contributor Author

Part of the work going on regarding the C++ modules migration of CMSSW (tracked as issue #15248). This PR is not meant to refactor things, but just to make these headers compile.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21412/2119

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Teemperor (Raphael Isemann) for master.

It involves the following packages:

TrackingTools/DetLayers

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @gpetruc, @ebrondol, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 21, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24574/console Started: 2017/11/21 16:43

@cmsbuild
Copy link
Contributor

-1

Tested at: da54493

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
893188d
0707ff5
bbddd85
ed2b195
12605de
3441962
c792deb
8d25c3d
84e2869
54ee550
3468977
eeb8263
32307fd
34b0dcf
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21412/24574/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21412/24574/git-merge-result

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

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test runtestTqafTopEventProducers had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafExamples had ERRORS

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
893188d
0707ff5
bbddd85
ed2b195
12605de
3441962
c792deb
8d25c3d
84e2869
54ee550
3468977
eeb8263
32307fd
34b0dcf
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21412/24574/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21412/24574/git-merge-result

@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-21412/24574/summary.html

There are some workflows for which there are errors in the baseline:
10824.0 step 5
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3523 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2746803
  • DQMHistoTests: Total failures: 9398
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2737228
  • DQMHistoTests: Total skipped: 177
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.920000000056 KiB( 23 files compared)
  • Checked 111 log files, 8 edm output root files, 27 DQM output files

@@ -1,3 +1,7 @@
#ifndef SIMPLE_STAT_H
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use an include guard matching the full file name subsystem_package_fileName_h

@perrotta
Copy link
Contributor

@Teemperor : please fix the include guard as requested in #21412 (review), so that this PR can get signed

@Teemperor
Copy link
Contributor Author

Done, sorry for the delay @perrotta

This header is lacking header guards and it uses std::max but
doesn't include the `algorithm` header. This path just adds
the missing guards and the right include to make this header
work with modules.
@perrotta
Copy link
Contributor

please test

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2017

@cmsbuild please test

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2017

code-checks

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2017

@smuzaffar @mrodozov
please check the bot.
It looks like some cleanup is needed to allow automated tests to proceed.

My two last requests were recognized by the bot
https://cmssdt.cern.ch/jenkins/job/cms-bot/385466/console
https://cmssdt.cern.ch/jenkins/job/cms-bot/385467/console
But neither of them have actually triggered any dependent tests.
My guess is the same has happened when @perrotta requested tests about a day ago.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21412/2239

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 29, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24743/console Started: 2017/11/29 13:17

@cmsbuild
Copy link
Contributor

Pull request #21412 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

@smuzaffar
Copy link
Contributor

@slava77 , looks like cms-bot jenkins job fail after commenting #21412 (comment) which means the code rules check job did not start and bot was waiting for it to complete. Anyway, I have updated the bot to re-trigger the code checks job when someone explicitly ask for it.

@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-21412/24743/summary.html

There are some workflows for which there are errors in the baseline:
10824.0 step 5
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2605338
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2605166
  • DQMHistoTests: Total skipped: 171
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.64000000013 KiB( 22 files compared)
  • Checked 106 log files, 8 edm output root files, 26 DQM output files

@perrotta
Copy link
Contributor

+1

  • Technical
  • No differences in jenkins tests: extra entries in the logError plots for wf 10224 are due to extra MB files open for PU

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2)

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 1194d70 into cms-sw:master Nov 30, 2017
@Teemperor Teemperor deleted the StandaloneStat branch December 19, 2017 08:47
@Teemperor Teemperor restored the StandaloneStat branch December 19, 2017 10:42
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.

6 participants