-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
L1TP2: Bug Fix: Adding a setting of TTTrack word to allow Track Quality MVA to read variables #44897
L1TP2: Bug Fix: Adding a setting of TTTrack word to allow Track Quality MVA to read variables #44897
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44897/40162
|
A new Pull Request was created by @BenjaminRS for master. It involves the following packages:
@cmsbuild, @epalencia, @aloeliger can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Please test |
Can we have some kind of plots to show before/after? Thx. |
@Chriisbrown FYI |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-166b27/39225/summary.html Comparison SummarySummary:
|
This is fine with me, I actually already included this fix into l1t-offline with the same solution. PR |
Is this PR still under discussion in L1T? @cms-sw/l1-l2 |
+l1 |
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, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
@cms-sw/l1-l2 |
@srimanob My immediate reaction would be no, this is just a bug fix for 14_1 and forward, but the developer doesn't think it would cause any harm. |
Hi @aloeliger Do your comment just come after fresh conversation, to correct PR description? |
Hi Phat and Andrew - I double checked. I was mistaken by saying that we needed a back port in the main CMSSW_14_0 branch (but we should do it in CMSSW_14_1). |
then please edit the description to remove the mention about it. |
PR description:
This is a bug fix to allow the Track Quality MVA to read variables which are in the TTrack word, before the Track Quality MVA is then set and saved back in the TTrack word.
PR validation:
This PR passes the following checks:
scram b
scram b code-format
scram b code-checks
Ran the L1TrackObjectNtupleMaker over a sample before and after the fix: the output of the MVA as seen when plotting trk_MVA1 now appears as desired
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
This will have to be back ported to the L1T OffSW Integration Branch too: Currently there is a PR by @cgsavard which performs the same fix
This needs to be backported to 14_0 to be used in the Phase2 MC production