cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard

KalmanFilter fix for Phase 2 L1 tracking

Open aehart opened this issue 1 year ago • 5 comments

@tomalin

PR description:

This PR resolves the failure reported in #44306. The problem was that matRinv is defined as a 2×2 matrix: https://github.com/cms-sw/cmssw/blob/CMSSW_14_0_0/L1Trigger/TrackFindingTMTT/src/KFbase.cc#L549 while unitMatrix is defined as a 5×5 matrix (nHelixPar_ is five when running the extended tracking): https://github.com/cms-sw/cmssw/blob/CMSSW_14_0_0/L1Trigger/TrackFindingTMTT/src/KFbase.cc#L554

So then there is an exception caused by assigning one to the other on this line: https://github.com/cms-sw/cmssw/blob/CMSSW_14_0_0/L1Trigger/TrackFindingTMTT/src/KFbase.cc#L556 Hence the error message:

…
[a] Fatal Root Error: @SUB=operator=(const TMatrixT &)
matrices not compatible

PR validation:

A recipe for reproducing the exception can be found in #44306. The changes in this PR allow the recipe to run without any exception.

aehart avatar Mar 15 '24 15:03 aehart

cms-bot internal usage

cmsbuild avatar Mar 15 '24 15:03 cmsbuild

Looks good to me.

tomalin avatar Mar 15 '24 15:03 tomalin

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44427/39500

  • This PR adds an extra 20KB to repository

cmsbuild avatar Mar 15 '24 15:03 cmsbuild

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

It involves the following packages:

  • L1Trigger/TrackFindingTMTT (l1)

@cmsbuild, @aloeliger, @epalencia can you please review it and eventually sign? Thanks. @erikbutz, @Martin-Grunewald, @skinnari, @missirol 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 Mar 15 '24 15:03 cmsbuild

Please test

epalencia avatar Mar 15 '24 15:03 epalencia

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a486eb/38172/summary.html COMMIT: fe18b411351e0b06dce81ddf0f6f219e66bea6fb CMSSW: CMSSW_14_1_X_2024-03-15-1100/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44427/38172/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:

  • @cms-tsg-storm cms-sw/cmssw#44415
  • @vlimant cms-sw/cmssw#44392
  • @haozturk cms-sw/cmssw#44381

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

Comparison Summary

Summary:

  • You potentially added 8 lines to the logs
  • Reco comparison results: 57 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3288140
  • DQMHistoTests: Total failures: 4687
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3283433
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1.092 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 12834.0,... ): -0.156 KiB HLT/Filters
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 7 / 46 workflows

cmsbuild avatar Mar 15 '24 21:03 cmsbuild

Hi @aehart @tomalin Is this consider as a bug-fix? So we can mark it in the PR. Thx.

srimanob avatar Mar 18 '24 06:03 srimanob

Hi @aehart @tomalin Is this consider as a bug-fix? So we can mark it in the PR. Thx.

Yes, this is a bug fix.

aehart avatar Mar 18 '24 12:03 aehart

type bug-fix

srimanob avatar Mar 18 '24 12:03 srimanob

+l1

aloeliger avatar Mar 18 '24 14:03 aloeliger

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

cmsbuild avatar Mar 18 '24 14:03 cmsbuild

+1

antoniovilela avatar Mar 18 '24 16:03 antoniovilela

@aehart Please backport to 14_0. Thanks.

srimanob avatar Mar 18 '24 16:03 srimanob