Skip to content

[Phase2 L1T] GMT Upgrade of Kalman Filter for Barrel Muon KMTF#51111

Open
zhenbinwu wants to merge 46 commits into
cms-sw:masterfrom
zhenbinwu:Kalman_26
Open

[Phase2 L1T] GMT Upgrade of Kalman Filter for Barrel Muon KMTF#51111
zhenbinwu wants to merge 46 commits into
cms-sw:masterfrom
zhenbinwu:Kalman_26

Conversation

@zhenbinwu

Copy link
Copy Markdown
Contributor

PR description:

  • Extend the Kalman Muon Barrel Track Finding algorithm for more precise measurements of track eta and z
  • Making use of new dataformat of Phase2 DT, Phi-Theta matched pairs
  • Detail in talk

PR validation:

Ongoing ...

@cmsbuild

cmsbuild commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

cms-bot internal usage

@cmsbuild

cmsbuild commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51111/49609

@cmsbuild

cmsbuild commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

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

It involves the following packages:

  • DataFormats/L1TMuonPhase2 (l1)
  • L1Trigger/Phase2L1GMT (l1)

@BenjaminRS, @cmsbuild, @quinnanm can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @eyigitba, @missirol, @mmusich, @rovere, @thomreis this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@quinnanm

quinnanm commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

please test

@cmsbuild

cmsbuild commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

+1

Size: This PR adds an extra 692KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-911f60/53636/summary.html
COMMIT: 6782bb3
CMSSW: CMSSW_17_0_X_2026-06-01-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/51111/53636/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 3 lines from the logs
  • Reco comparison results: 16 differences found in the comparisons
  • DQMHistoTests: Total files compared: 53
  • DQMHistoTests: Total histograms compared: 4199113
  • DQMHistoTests: Total failures: 392
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4198701
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 52 files compared)
  • Checked 227 log files, 197 edm output root files, 53 DQM output files
  • TriggerResults: found differences in 3 / 51 workflows

@cmsbuild

cmsbuild commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Milestone for this pull request has been moved to CMSSW_20_0_X. Please open a backport if it should also go in to CMSSW_17_0_X.

@cmsbuild cmsbuild modified the milestones: CMSSW_17_0_X, CMSSW_20_0_X Jun 5, 2026
@makortel

makortel commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

(I assume this PR does not need a backport to 17_0_X (Run 3 legacy))

Comment on lines +29 to +30
<version ClassVersion="5" checksum="251130338"/>
<version ClassVersion="4" checksum="251130338"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one new class version should be added. Very likely you also need to rebase on top of 20_0_0_pre1 (hopefully available early next week) because the checksum for version 3 will be changed for that.

@BenjaminRS

Copy link
Copy Markdown
Contributor

HI Matti - indeed this is a Phase 2 only algorithm for 20_1_X

@quinnanm quinnanm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR! Just a few minor comments.

int KMTFCore::customBitmask(unsigned int bit1, unsigned int bit2, unsigned int bit3, unsigned int bit4) {
return bit1 * 1 + bit2 * 2 + bit3 * 4 + bit4 * 8;
}

bool KMTFCore::getBit(int bitmask, int pos) { return (bitmask & (1 << pos)) >> pos; }

void KMTFCore::setFourVectors(l1t::KMTFTrack& track) {
int etaINT = track.coarseEta();
//int etaINT = track.coarseEta();
ap_ufixed<18, 0> m = 0.03602;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you maybe add a comment adding context to these numbers?

@@ -745,7 +1045,7 @@ bool KMTFCore::updateLUT(l1t::KMTFTrack& track, const l1t::MuonStubRef& stub, in
<< ap_ufixed<GAIN2_5, GAIN2_5INT>(GAIN[3]).to_float();
}

track.setKalmanGain(track.step(), fabs(trackK), GAIN[0], GAIN[1], 1, 0, GAIN[2], GAIN[3]);
//track.setKalmanGain(track.step(), fabs(trackK), GAIN[0], GAIN[1], 1, 0, GAIN[2], GAIN[3]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this comment be deleted?

@@ -1086,7 +1464,7 @@ int KMTFCore::encode(bool ownwheel, int sector, int tag) {

std::pair<bool, uint> KMTFCore::getByCode(const std::vector<l1t::KMTFTrack>& tracks, int mask) {
for (uint i = 0; i < tracks.size(); ++i) {
edm::LogInfo("KMTFCore") << "Code=" << tracks[i].hitPattern() << ", track=" << mask;
//edm::LogInfo("KMTFCore") << "Code=" << tracks[i].hitPattern() << ", track=" << mask;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this comment be deleted?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the intended location of this file or is it planned to live elsewhere like cms-data?

Comment on lines 169 to +170
TFile *lutFile_;
TFile *lutThetaFile_;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be std::unique_ptr<TFile>?

const auto& phiS = pairs.phiDigi();
static constexpr int ZCenterDigitizedPositive[] = {11730, 23327};
static constexpr int ZCenterDigitizedNegative[] = {-11697, -23290};
static constexpr int ZCenterDigitizedZero[] = {22};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit odd as a single element array rather than a constexpr int but if you really like it this way I guess it's fine

@@ -110,12 +134,51 @@ namespace Phase2L1GMT {
return uint((1 << 12) * coarseEta_->GetBinContent(coarseEta_->GetXaxis()->FindBin(mask)) / M_PI);
}

std::vector<float> trackGainTheta(uint step, uint bitmask, uint K, bool is2D) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be const? likewise the trackGainTheta2 getter

@cmsbuild

cmsbuild commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Milestone for this pull request has been moved to CMSSW_20_1_X. Please open a backport if it should also go in to CMSSW_20_0_X.

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