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

Log the number of Streams since it might be different than the number of Threads #16372

Merged
merged 3 commits into from Oct 29, 2016
Merged

Log the number of Streams since it might be different than the number of Threads #16372

merged 3 commits into from Oct 29, 2016

Conversation

gartung
Copy link
Member

@gartung gartung commented Oct 27, 2016

I found this to be missing when running benchmarks on the KNL system.

@gartung
Copy link
Member Author

gartung commented Oct 27, 2016

@Dr15Jones @smuzaffar

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gartung (Patrick Gartung) for CMSSW_8_1_DEVEL_X.

It involves the following packages:

FWCore/Framework

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit, @wmtan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@@ -434,6 +434,8 @@ namespace edm {
nStreams = nThreads;
}
}
// PG: Log the number of streams
edm::LogInfo("StreamSetup") <<"setting # streams "<<nStreams;
Copy link
Contributor

Choose a reason for hiding this comment

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

Only print number of streams if the option was explicitly set. That would be moving this to be between line 435 and 436.

@gartung
Copy link
Member Author

gartung commented Oct 27, 2016

@Dr15Jones OK I moved the logging location.

@Dr15Jones
Copy link
Contributor

please test

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 27, 2016

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

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_DEVEL_X IBs after it passes the integration tests.

@cmsbuild
Copy link
Contributor

-1

Tested at: bf70879

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

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test TestFWCoreFrameworkGlobalStreamOne had ERRORS

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Pull request #16372 was updated. @cmsbuild, @smuzaffar, @Dr15Jones can you please check and sign again.

@cmsbuild
Copy link
Contributor

@gartung
Copy link
Member Author

gartung commented Oct 27, 2016

@Dr15Jones please retest.

@Dr15Jones
Copy link
Contributor

please test

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 27, 2016

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

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_DEVEL_X IBs after it passes the integration tests.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@smuzaffar
Copy link
Contributor

@davidlange6 , bot adds ORP signatures for release based on normal branch (e.g. CMSSW_8_1_X). This PR is for DEVEL IB that is why bot did not add ORP. Should I change it so that it adds ORP signs for all PRs?

@davidlange6
Copy link
Contributor

ah - I missed that.. yes- lets add orp for the devel build as well as we will build the next release cycle from it

On Oct 28, 2016, at 11:23 AM, Malik Shahzad Muzaffar notifications@github.com wrote:

@davidlange6 , bot adds ORP signatures for release based on normal branch (e.g. CMSSW_8_1_X). This PR is for DEVEL IB that is why bot did not add ORP. Should I change it so that it adds ORP signs for all PRs?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@smuzaffar smuzaffar merged commit a36180b into cms-sw:CMSSW_8_1_DEVEL_X Oct 29, 2016
@gartung gartung deleted the FWCore-Framework-log-nStreams branch December 1, 2016 02:21
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

5 participants