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
Require highPurity for isolated tracks in heavy-ion workflows #31727
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31727/18924
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31727/18926
|
A new Pull Request was created by @mandrenguyen (Matthew Nguyen) for master. It involves the following packages: PhysicsTools/PatAlgos @perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@jpata It doesn't appear any downstream modules in miniAOD consume isolatedTracks. I'm not really sure what this collection is used for, honestly. I do see in LXR that it's used in a muon POG skim, perhaps they are the main customer? Presumably no one has ever used this collection in PbPb events, since we haven't yet ever produced miniAOD. |
OK, thanks for the clarifications. In this case, I only see a possible issue that it's adding extra functionality to the PATIsolatedTrackProducer which could perhaps also be achieved in the HI workflows by adding a PATObjectSelector in front of isolatedTracks. This would help to decouple functionality between various modules and perhaps be more modular. |
IIRC, isolatedTracks from the start had somewhat elaborate selections in the producer itself. |
@jpata We create a collection called highPurityGeneralTracks using TrackSelector. However, when I use it as input to isolatedTracks, I get the error I believe that comes from the fact that particle flow uses the full generalTracks as input. |
It looks like you can hand it a string as a selection, but that's only for the dE/dx part of the code. |
OK good, thanks for the checks, then I see no reason to go forward as implemented and tested already here. |
+reconstruction
|
+1 |
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
In #31647 it was pointed out that isolatedTracks has a high CPU consumption in central heavy-ion events.
This PR requires that only highPurity tracks are considered, which cuts the timing by a factor of 4 in wf 140.5611
Non-highPurity tracks have a rather large fake rate and are not used for many purposes.
PR validation:
if this PR is a backport please specify the original PR and why you need to backport that PR:
Before submitting your pull requests, make sure you followed this checklist: