cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard

Published 20 hours ago • verified publisher icon cms-sw

Z vertex multi layout

Open ericcano opened this issue 1 year ago • 31 comments

PR description:

This PR splits ZVertexSoA into two layouts inside a multi collection: one related to vertices, and one related to tracks. ~~It requires #40285 which is merged but not in any IB yet,~~

This is currently a direct translation of the code. As stated in its comment, the field ndof is sometimes used with vertex indices (and hence not used fully). This could be reviewed separately.

PR validation:

All unit tests from affected modules pass.

ericcano avatar Feb 13 '24 16:02 ericcano

This PR cannot be merged in master so I will rebase it.

ericcano avatar Feb 13 '24 16:02 ericcano

cms-bot internal usage

cmsbuild avatar Feb 13 '24 16:02 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38829

ERROR: Unable to merge PR.

See log https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38829/cms-checkout-topic.log

cmsbuild avatar Feb 13 '24 16:02 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38885

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

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

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38885/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38885/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Feb 15 '24 11:02 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38886

  • This PR adds an extra 56KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 15 '24 11:02 cmsbuild

A new Pull Request was created by @ericcano (Eric Cano) for master.

It involves the following packages:

  • DQM/SiPixelHeterogeneous (dqm)
  • DataFormats/VertexSoA (reconstruction, heterogeneous)
  • RecoTauTag/HLTProducers (hlt)
  • RecoTracker/PixelTrackFitting (reconstruction)
  • RecoTracker/PixelVertexFinding (reconstruction)

@jfernan2, @mmusich, @antoniovagnerini, @nothingface0, @tjavaid, @cmsbuild, @makortel, @Martin-Grunewald, @syuvivida, @rvenditti, @fwyzard, @mandrenguyen can you please review it and eventually sign? Thanks. @felicepantaleo, @missirol, @azotz, @dgulhan, @gpetruc, @JanFSchulte, @mmusich, @jandrea, @silviodonato, @VinInn, @mtosi, @mbluj, @threus, @rovere, @GiacomoSguazzoni, @fioriNTU, @idebruyn, @VourMa this is something you requested to watch as well. @sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

cmsbuild avatar Feb 15 '24 11:02 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38889

  • This PR adds an extra 56KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 15 '24 13:02 cmsbuild

Pull request #43952 was updated. @fwyzard, @cmsbuild, @rvenditti, @tjavaid, @mmusich, @Martin-Grunewald, @mandrenguyen, @makortel, @antoniovagnerini, @syuvivida, @nothingface0, @jfernan2 can you please check and sign again.

cmsbuild avatar Feb 15 '24 13:02 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38892

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 15 '24 14:02 cmsbuild

Pull request #43952 was updated. @rvenditti, @makortel, @tjavaid, @syuvivida, @Martin-Grunewald, @mmusich, @nothingface0, @jfernan2, @antoniovagnerini, @mandrenguyen, @cmsbuild, @fwyzard can you please check and sign again.

cmsbuild avatar Feb 15 '24 14:02 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38896

  • This PR adds an extra 36KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 15 '24 17:02 cmsbuild

Pull request #43952 was updated. @makortel, @jfernan2, @syuvivida, @Martin-Grunewald, @nothingface0, @cmsbuild, @antoniovagnerini, @fwyzard, @mandrenguyen, @mmusich, @rvenditti, @tjavaid can you please check and sign again.

cmsbuild avatar Feb 15 '24 17:02 cmsbuild

please test

fwyzard avatar Feb 15 '24 23:02 fwyzard

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e87815/37511/summary.html COMMIT: c0e29953e7cce0ef329a8268e64f5b783ae586be CMSSW: CMSSW_14_1_X_2024-02-15-1100/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43952/37511/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

cmsbuild avatar Feb 16 '24 05:02 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38916

  • This PR adds an extra 36KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 16 '24 13:02 cmsbuild

Pull request #43952 was updated. @nothingface0, @Martin-Grunewald, @makortel, @mmusich, @syuvivida, @tjavaid, @jfernan2, @cmsbuild, @mandrenguyen, @rvenditti, @fwyzard, @antoniovagnerini can you please check and sign again.

cmsbuild avatar Feb 16 '24 13:02 cmsbuild

~~From a quick test, these changes save about 10MB of GPU memory on a standard HLT job (32 threads/24 stream). Not much, but the effect is visible.~~

Scratch that, it's probably within the noise... re-running the jobs gave pretty different results :-/

fwyzard avatar Feb 16 '24 17:02 fwyzard

If I got it right, the earlier memory need would be ~768 kB / event, that the CachingAllocator would then round up to 1 MB. With this PR the memory need would be ~210 kB, that the CachingAllocator would then rounds up to 256 kB. The saving would then be 768 kB/event, that would correspond to 18 MB for 24 streams (assuming every event would have the vertex data product).

(and I can completely believe the dynamic behavior of the job could wash out that difference, at least in individual tests)

makortel avatar Feb 16 '24 19:02 makortel

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/38934

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Feb 19 '24 10:02 cmsbuild

Pull request #43952 was updated. @mandrenguyen, @fwyzard, @syuvivida, @antoniovagnerini, @makortel, @jfernan2, @Martin-Grunewald, @tjavaid, @nothingface0, @cmsbuild, @mmusich, @rvenditti can you please check and sign again.

cmsbuild avatar Feb 19 '24 10:02 cmsbuild

is this ready for another round of tests?

mmusich avatar Feb 22 '24 08:02 mmusich

maybe ?

fwyzard avatar Feb 22 '24 09:02 fwyzard

please test

fwyzard avatar Feb 22 '24 09:02 fwyzard

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e87815/37622/summary.html COMMIT: f8270977e2efa1282bc8daa1b13792dbbd2ae24d CMSSW: CMSSW_14_1_X_2024-02-21-2300/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43952/37622/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

cmsbuild avatar Feb 22 '24 15:02 cmsbuild

code-checks

mmusich avatar Mar 05 '24 15:03 mmusich

this branch has conflicts to resolve @ericcano FYI

mmusich avatar Mar 05 '24 15:03 mmusich

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39341

ERROR: Unable to merge PR.

See log https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39341/cms-checkout-topic.log

cmsbuild avatar Mar 05 '24 15:03 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39433

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

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

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39433/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39433/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Mar 12 '24 12:03 cmsbuild

The conflicts were fixed. They only impacted the unit test.

ericcano avatar Mar 12 '24 12:03 ericcano

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43952/39434

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File DQM/SiPixelHeterogeneous/plugins/SiPixelCompareVertexSoAAlpaka.cc modified in PR(s): #43964
    • File RecoTauTag/HLTProducers/src/L2TauTagNNProducerAlpaka.cc modified in PR(s): #43964
    • File RecoTracker/PixelVertexFinding/plugins/alpaka/vertexFinder.dev.cc modified in PR(s): #43964

cmsbuild avatar Mar 12 '24 12:03 cmsbuild