Skip to content

Conversation

@aloeliger
Copy link
Contributor

PR description:

This PR now takes the place of #41260 which messed up the git history, and supersedes the original PR #40670

Previous PR description:

This is an update of the correlator code

Includes:

p2l1pfp#92 by @thesps and @slaurila
p2l1pfp#94 by @thomreis and @cerminar (only adds a new feature to the emulator, doesn't change any of the existing emulated values)
p2l1pfp#96 by @thesps
p2l1pfp#97 by @drankincms
p2l1pfp#98 by @drankincms (not used by default)
p2l1pfp#100 by @thesps
p2l1pfp#101 by @gpetruc (not used in production)
p2l1pfp#105 by @gpetruc (not used in production)
p2l1pfp#107 by @gpetruc (transparent)

Local: cms-l1t-offline#1063

My apologies to the original authors, but the code has been squashed to remove history. I have added everyone as co-authors to the squashed commit, but it may not be your github email address.

I will be updating this PR as requested in the original thread and the messed up PR, as soon as I am able.

EmyrClement and others added 2 commits April 3, 2023 11:36
Co-authored-by: Sioni Summers <sioni.paris.summers@cern.ch>
Co-authored-by: Santeri Laurila <santeri.laurila@cern.ch>
Co-authored-by: Thomas Reis <thomas.reis@cern.ch>
Co-authored-by: Gianluca Cerminara <gianluca.cerminara@cern.ch>
Co-authored-by: Dylan Rankin <dylan.sheldon.rankin@cern.ch>
Co-authored-by: Giovanni Petrucciani <giovanni.petrucciani@cern.ch>
Co-authored-by: Emyr Clement <emyr.john.clement@cern.ch>
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41279/35046

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

A new Pull Request was created by @aloeliger (Andrew Loeliger) for master.

It involves the following packages:

  • DataFormats/L1TParticleFlow (upgrade, l1)
  • L1Trigger/Configuration (l1)
  • L1Trigger/Phase2L1ParticleFlow (upgrade, l1)

@epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @rovere this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

Add SC jets from extended tracks to event content.
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41279/35050

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

Pull request #41279 was updated. @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please check and sign again.

@aloeliger
Copy link
Contributor Author

aloeliger commented Apr 5, 2023

@makortel In reference to: #40670 (comment), could you look over my shoulder a bit on a22c934 and make sure this solves your comment adequately. This should put the session at the cache level and reference it at the ID level for running of the NN, but I'm tossing around references to pointers, which seems like the sort of thing one should double check with somebody.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41279/35052

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2023

Pull request #41279 was updated. @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41279/35332

@cmsbuild
Copy link
Contributor

Pull request #41279 was updated. @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please check and sign again.

@aloeliger
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-add0e4/32275/summary.html
COMMIT: 46d37af
CMSSW: CMSSW_13_1_X_2023-05-01-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41279/32275/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 8 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 11 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3460877
  • DQMHistoTests: Total failures: 67
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3460788
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 3 / 46 workflows

@aloeliger
Copy link
Contributor Author

+l1

@AdrianoDee
Copy link
Contributor

+upgrade
(comments addressed, there's this only left but to be honest I can't really say it would be blocking from my side)

@cmsbuild
Copy link
Contributor

cmsbuild commented May 3, 2023

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

@perrotta
Copy link
Contributor

perrotta commented May 3, 2023

@smuzzafar, do I remember correctly that there was a way to get the output of the static analyzer even if it is not accessible through the usual link in #41279 (comment) ?

@smuzaffar smuzaffar modified the milestones: CMSSW_13_1_X, CMSSW_13_2_X May 4, 2023
cmsbuild added a commit to cms-data/L1Trigger-Phase2L1ParticleFlow that referenced this pull request May 4, 2023
Update files that have been removed in cms-sw/cmssw#41279
@perrotta
Copy link
Contributor

perrotta commented May 4, 2023

+1

  • It is a pity that we cannot access the results of the static analyzer, because this PR was somehow messed up before the last commits, and those warnings can be useful to spot possible hidden mistakes in the logic. Let keep an eye on those outputs in the IBs, when available

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.