-
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
Introduce PtMaxTrackCountFilter
#37500
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37500/29202
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
a3e1078
to
025780f
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37500/29203
|
A new Pull Request was created by @mmusich (Marco Musich) for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c26816/23746/summary.html Comparison SummarySummary:
|
Hi @clacaputo, can you clarify what's holding this PR? shall unit tests be implemented here directly as a conditio sine qua non for accepting it? |
Hi @mmusich,
if this is critical and it's needed by the next release, we can merge the PR as it is and have the unit test in a subsequent one. |
Since I would push the changes exercising the new module in a different package, together with other changes (not under reco signature) I would prefer to keep it separate, unless there's a strong push (to get you an idea of where it would be used you can look here). |
+reconstruction
|
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, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c26816/23934/summary.html Comparison SummarySummary:
|
+1 |
for the record, here it is: #37621 |
PR description:
To study cosmic commissioning events (especially to check detector noise), it is sometimes convenient to isolate low pT tracks.
As far as I could see, there's no such filter to retain only events with at least a track below a certain threshold. This is added here.
In addition I profit to fix the
fillDescriptions
method of PtMinTrackCountFilter.cc which has been introduced in PR #29604 (this is done in commit: a3e1078, without it, using the filter results in failure validating the configuration).PR validation:
Private, tested adding:
to configuration file and tested it works correctly.
if this PR is a backport please specify the original PR and why you need to backport that PR:
N/A