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

Make the the size of the binner (HistoContainer) settable at run time. The total number of Pixel Clusters is not a limit anymore on GPU #33371

Merged
merged 9 commits into from
Apr 16, 2021

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Apr 8, 2021

port of cms-patatrack#590.

It makes LocalReco independent from the size of the event and tuple-making independent of the number of hits.
Some of the containers used by the CA will remain of fixed size (cannot be extended during the pattern recognition).

purely technical. no regression expected (unless the total number of clusters was larger than previous limit).

Further needs by HI and Phase2 will be adressed in future PRs

This makes the pixel local reconstruction independent from the size of the event, and tuple-making
independent of the number of hits.

Some of the containers used by the CA remain of fixed size, as they cannot be extended during the
pattern recognition).
@VinInn
Copy link
Contributor Author

VinInn commented Apr 8, 2021

@cmsbuild please test

@VinInn VinInn changed the title Make the the size of the binner (HistoContainer) settable at run time. The total number of Hits is not a limit anymore. Make the the size of the binner (HistoContainer) settable at run time. The total number of Pixel Clusters is not a limit anymore on GPU Apr 8, 2021
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33371/21961

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2021

A new Pull Request was created by @VinInn (Vincenzo Innocente) for master.

It involves the following packages:

CUDADataFormats/SiPixelCluster
CUDADataFormats/TrackingRecHit
HeterogeneousCore/CUDAUtilities
RecoLocalTracker/SiPixelClusterizer
RecoLocalTracker/SiPixelRecHits
RecoPixelVertexing/PixelTriplets

@perrotta, @makortel, @jpata, @fwyzard, @slava77 can you please review it and eventually sign? Thanks.
@mtosi, @makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @OzAmram, @ferencek, @dkotlins, @gpetruc, @ebrondol, @threus, @dgulhan, @tvami this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@fwyzard
Copy link
Contributor

fwyzard commented Apr 8, 2021

please test

@fwyzard
Copy link
Contributor

fwyzard commented Apr 8, 2021

enable gpu

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7401c4/14093/summary.html
COMMIT: 37bb68e
CMSSW: CMSSW_11_3_X_2021-04-07-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/33371/14093/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: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 4
  • DQMHistoTests: Total histograms compared: 9575
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 9575
  • 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: 38
  • DQMHistoTests: Total histograms compared: 2865506
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2865477
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@fwyzard

This comment has been minimized.

@fwyzard
Copy link
Contributor

fwyzard commented Apr 12, 2021

I'd like to retest this after #31854 is merged, to check also the triplets.

@VinInn
Copy link
Contributor Author

VinInn commented Apr 14, 2021

ok. So we are ready to merge, aren't we?

@VinInn VinInn changed the base branch from master to CMSSW_11_3_X April 14, 2021 08:15
@cmsbuild cmsbuild changed the base branch from CMSSW_11_3_X to master April 14, 2021 08:15
@cmsbuild
Copy link
Contributor

@VinInn, CMSSW_11_3_X branch is closed for direct updates. cms-bot is going to move this PR to master branch.
In future, please use cmssw master branch to submit your changes.

@srimanob
Copy link
Contributor

As mentioned by @slava77, should this PR be rebased first? I see several of them are already merged with #31854

Thanks.

@VinInn VinInn changed the base branch from master to CMSSW_11_3_X April 14, 2021 08:15
@cmsbuild cmsbuild changed the base branch from CMSSW_11_3_X to master April 14, 2021 08:15
@cmsbuild
Copy link
Contributor

@VinInn, CMSSW_11_3_X branch is closed for direct updates. cms-bot is going to move this PR to master branch.
In future, please use cmssw master branch to submit your changes.

@VinInn
Copy link
Contributor Author

VinInn commented Apr 14, 2021

should be ok now

@fwyzard
Copy link
Contributor

fwyzard commented Apr 14, 2021

+heterogeneous

@slava77
Copy link
Contributor

slava77 commented Apr 14, 2021

+reconstruction

for #33371 9420952

  • in line with the PR title
  • jenkins tests pass and comparisons with the baseline in GPU and CPU tests show no differences, as expected

@VinInn please copy relevant details of cms-patatrack#590 in this PR description to improve direct self-documentation

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

@VinInn
Copy link
Contributor Author

VinInn commented Apr 15, 2021

can this be merged before the weekend?

@cmsbuild cmsbuild modified the milestones: CMSSW_11_3_X, CMSSW_12_0_X Apr 15, 2021
@qliphy
Copy link
Contributor

qliphy commented Apr 16, 2021

+1

This pull request was closed.
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