Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enable mkFit initialStep for track building in phase-2 era, and introduce noMkFit phase-2 era(s) #47383

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

mmasciov
Copy link
Contributor

PR description:
As per title, and presented at TRK POG meeting on Feb. 11:
https://indico.cern.ch/event/1498184/#56-mkfit-updates-for-phase-2

PR validation:
As presented at TRK POG meeting on Feb. 11:
https://indico.cern.ch/event/1498184/#56-mkfit-updates-for-phase-2

In short: physics performance is ~ on par with CKF (default), while timing is reduced.

It requires PR cms-data/RecoTracker-MkFit#16 for results shown at TRK POG.

FYI @kskovpen @slava77

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 18, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47383/43756

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/StandardSequences (operations)

@antoniovilela, @cmsbuild, @davidlange6, @fabiocos, @mandrenguyen, @rappoccio can you please review it and eventually sign? Thanks.
@AnnikaStein, @GiacomoSguazzoni, @Martin-Grunewald, @VinInn, @VourMa, @dgulhan, @fabiocos, @felicepantaleo, @makortel, @missirol, @mmusich, @mtosi, @rovere, @sameasy, @slomeo this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@mmasciov
Copy link
Contributor Author

test parameters:

pull_request = cms-data/RecoTracker-MkFit#16

@mmasciov
Copy link
Contributor Author

type tracking

@mmasciov
Copy link
Contributor Author

enable profiling

@mmasciov
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: ClangBuild
Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/summary.html
COMMIT: 2cc100f
CMSSW: CMSSW_15_1_X_2025-02-17-2300/el8_amd64_gcc12
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47383/44467/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/git-merge-result

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' /usr/bin/time -v scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

@mmasciov
Copy link
Contributor Author

mmasciov commented Feb 18, 2025

I believe the error/warning (https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/clang-new-warnings.log) seems fully unrelated to this PR, and I can't reproduce it myself. Should I just try to relaunch the tests? (@cms-sw/operations-l2, @cms-sw/orp-l2)

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2025

I believe the error/warning (https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/clang-new-warnings.log) seems fully unrelated to this PR, and I can't reproduce it myself. Should I just try to relaunch the tests? (@cms-sw/operations-l2, @cms-sw/orp-l2)

unless you get a very quiet IB (no merges from an IB to the start of the tests), I think the cms-data part should be merged first and then the tests rerun; otherwise the bot pick up everything from the master in addition to the PR during the tests.

@mmasciov
Copy link
Contributor Author

please test

@mmasciov
Copy link
Contributor Author

I believe the error/warning (https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44467/clang-new-warnings.log) seems fully unrelated to this PR, and I can't reproduce it myself. Should I just try to relaunch the tests? (@cms-sw/operations-l2, @cms-sw/orp-l2)

unless you get a very quiet IB (no merges from an IB to the start of the tests), I think the cms-data part should be merged first and then the tests rerun; otherwise the bot pick up everything from the master in addition to the PR during the tests.

Thanks! I launched the tests for the cms-data PR, and relaunched this too (just in case random errors do not show up).

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44513/summary.html
COMMIT: 2cc100f
CMSSW: CMSSW_15_1_X_2025-02-19-1100/el8_amd64_gcc12
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47383/44513/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44513/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-571dc6/44513/git-merge-result

Comparison Summary

Summary:

@mmasciov
Copy link
Contributor Author

Tests have passed, for both this and cms-data/RecoTracker-MkFit#16 (analogous results, as it should be). @cms-sw/operations-l2 and @cms-sw/orp-l2, I believe these two are now in your hands. Thanks!

@slava77
Copy link
Contributor

slava77 commented Feb 21, 2025

Tests have passed, for both this and cms-data/RecoTracker-MkFit#16 (analogous results, as it should be). @cms-sw/operations-l2 and @cms-sw/orp-l2, I believe these two are now in your hands. Thanks!

should this be checked by reconstruction as well? A relevant L2 would need to assign.

@mandrenguyen
Copy link
Contributor

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

@jfernan2,@mandrenguyen you have been requested to review this Pull request/Issue and eventually sign? Thanks

@jfernan2
Copy link
Contributor

+1

@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. @sextonkennedy, @rappoccio, @antoniovilela, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: cms-data/RecoTracker-MkFit#16

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 38e3ea5 into cms-sw:master Feb 21, 2025
14 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants