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

HBHEGPU: Channel quality gpu [master] #35357

Merged
merged 4 commits into from Sep 28, 2021

Conversation

mariadalfonso
Copy link
Contributor

forward port of #35355

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35357/25424

  • This PR adds an extra 36KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35357/25425

  • This PR adds an extra 40KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mariadalfonso for master.

It involves the following packages:

  • CondFormats/HcalObjects (db, alca)
  • RecoLocalCalo/HcalRecProducers (reconstruction)

@malbouis, @yuanchao, @cmsbuild, @jpata, @slava77, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@apsallid, @tocheng, @bsunanda, @mmusich, @abdoulline, @seemasharmafnal this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@mariadalfonso mariadalfonso changed the title HBHEGPU: Channel quality gpu master HBHEGPU: Channel quality gpu [master] Sep 21, 2021
@fwyzard
Copy link
Contributor

fwyzard commented Sep 22, 2021

enable gpu

@fwyzard
Copy link
Contributor

fwyzard commented Sep 22, 2021

please test

@tvami
Copy link
Contributor

tvami commented Sep 22, 2021

enable gpu

@fwyzard I think you missed this
#35355 (comment)
(i.e. adding 11634.522 -- unless it's in there in default)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca68b4/18830/summary.html
COMMIT: bf1bab9
CMSSW: CMSSW_12_1_X_2021-09-21-2300/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35357/18830/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 4
  • DQMHistoTests: Total histograms compared: 19735
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 19722
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
  • Checked 12 log files, 9 edm output root files, 4 DQM output files
  • TriggerResults: no differences found

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3211080
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3211052
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca68b4/18859/summary.html
COMMIT: 58b842e
CMSSW: CMSSW_12_1_X_2021-09-23-1100/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35357/18859/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 4
  • DQMHistoTests: Total histograms compared: 19735
  • DQMHistoTests: Total failures: 28
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 19707
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
  • Checked 12 log files, 9 edm output root files, 4 DQM output files
  • TriggerResults: no differences found

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 40
  • DQMHistoTests: Total histograms compared: 3211080
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3211046
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 39 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 169 log files, 37 edm output root files, 40 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor Author

I've tested this PR as described in #35376, and while it significantly reduces the HCAL-related discrepancies, it does not eliminate them completely.

I suggest to finalise the review of this and merge it.
I will look after the residual discrepancies.

@fwyzard
Copy link
Contributor

fwyzard commented Sep 27, 2021

Sounds like a good idea to me.

@jpata
Copy link
Contributor

jpata commented Sep 27, 2021

+reconstruction

  • sync Mahi on GPU with what is done on CPU, to solve (but not completely eliminate) differences in the Run3 HLT menu
  • minor differences in GPU workflows for HBHERecHitsSorted are observed, this is expected

@Martin-Grunewald
Copy link
Contributor

+1

@tvami
Copy link
Contributor

tvami commented Sep 27, 2021

I suggest to finalise the review of this and merge it.

I've not seen my comments addressed: #35355 (comment)

@mariadalfonso
Copy link
Contributor Author

I suggest to finalise the review of this and merge it.

I've not seen my comments addressed: #35355 (comment)

well, no way this PR affects the Pixel aligment.
Are you sure the test didn't pick up some other changes ?

@tvami
Copy link
Contributor

tvami commented Sep 27, 2021

I suggest to finalise the review of this and merge it.

I've not seen my comments addressed: #35355 (comment)

well, no way this PR affects the Pixel aligment.
Are you sure the test didn't pick up some other changes ?

@mariadalfonso yes, I've resubmitted the tests several times and got the same results, and then the same thing is there in the master and the backport.

@mariadalfonso
Copy link
Contributor Author

I suggest to finalise the review of this and merge it.

I've not seen my comments addressed: #35355 (comment)

well, no way this PR affects the Pixel aligment.
Are you sure the test didn't pick up some other changes ?

@mariadalfonso yes, I've resubmitted the tests several times and got the same results, and then the same thing is there in the master and the backport.

Do these spurious GPU-"Pixel alignment" plots appears also in previous PR's test ? maybe there is something not reproducible?

@tvami
Copy link
Contributor

tvami commented Sep 27, 2021

I suggest to finalise the review of this and merge it.

I've not seen my comments addressed: #35355 (comment)

well, no way this PR affects the Pixel aligment.
Are you sure the test didn't pick up some other changes ?

@mariadalfonso yes, I've resubmitted the tests several times and got the same results, and then the same thing is there in the master and the backport.

Do these spurious GPU-"Pixel alignment" plots appears also in previous PR's test ?

Not that I know of, i.e. nothing that touches AlCa. Maybe @cms-sw/reconstruction-l2 sees them in other PRs?

@slava77
Copy link
Contributor

slava77 commented Sep 27, 2021

Do these spurious GPU-"Pixel alignment" plots appears also in previous PR's test ?

Not that I know of, i.e. nothing that touches AlCa. Maybe @cms-sw/reconstruction-l2 sees them in other PRs?

some of the pixel GPU reco is not repeatable

@tvami
Copy link
Contributor

tvami commented Sep 27, 2021

+db

@tvami
Copy link
Contributor

tvami commented Sep 27, 2021

+alca

  • differences are understood
  • differences in Pixel wfs on GPUs are due to the fact that "some of the pixel GPU reco is not repeatable" see above

@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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Sep 28, 2021

+1

@cmsbuild cmsbuild merged commit 214a72f into cms-sw:master Sep 28, 2021
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

8 participants