Cleanup pixel tracking code (master) #24288
Conversation
Can be included with the following snippet in the configuration:
from RecoPixelVertexing.Configuration.customizePixelTracksForProfiling import customizePixelTracksForProfiling
process = customizePixelTracksForProfiling(process)
Removes validation, DQM, and output modules.
As suggested in cms-patatrack#70 (comment), an `AsciiOutputModule` is used to require the `pixelTracks`.
Backport from the Patatrack fork (cms-patatrack#106).
|
The code-checks are being triggered in jenkins. |
|
@cmsbuild, please test |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24288/6021 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24288/6021/git-diff.patch You can run |
Cleanup whitespaces and indenttion, plugin definitions, includes, and file names. Backport from the Patatrack fork: cms-patatrack#49, cms-patatrack#122, cms-patatrack#134.
fwyzard
force-pushed
the
cleanup_pixel_tracking_code_103x
branch
from
417c50c to
5622c7a
Compare
|
@cmsbuild, please test |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24288/6023 |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @fwyzard (Andrea Bocci) for master. It involves the following packages: EventFilter/SiPixelRawToDigi @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| } | ||
|
|
||
| void CellularAutomaton::evolve(const unsigned int minHitsPerNtuplet) | ||
| { | ||
| allStatus.resize(allCells.size()); | ||
|
|
||
|
|
||
|
|
||
| unsigned int numberOfIterations = minHitsPerNtuplet - 2; |
There was a problem hiding this comment.
Ok, minHitsPerNtuplet is defined constant to "4" in the calling class in
http://cmslxr.fnal.gov/source/RecoPixelVertexing/PixelTriplets/src/CAHitQuadrupletGenerator.cc#0188
However, I find potentially dangerous to make differences of unsigned int's: is it fixed by the logic of the code that numberOfIterations cannot ever be passed as less or equal 2?
(Not strictly related to your PR, but I hit it while reviewing and I think it is worth to ask)
There was a problem hiding this comment.
I wouldn't know...
@felicepantaleo can you comment ?
There was a problem hiding this comment.
ciao Andrea,
The evolve method is and will never be called if the number of hits is smaller than 4 (you can check the triplet generator which does not call it).
If you really really want we can add an assert, but the assumption that the number of hits is equal to 4 is visible everywhere in: http://cmslxr.fnal.gov/source/RecoPixelVertexing/PixelTriplets/src/CAHitQuadrupletGenerator.cc
| #include<cmath> | ||
|
|
||
|
|
||
| #include "RecoTracker/TkSeedingLayers/interface/SeedingHitSet.h" |
There was a problem hiding this comment.
This include wasn't alphabetically ordered as all the other ones.
Just pointing it out to you in case you didn't notice it: I really don't care too much about such an alphabetical order
There was a problem hiding this comment.
Actually, it looks to me like it is ?
https://github.com/cms-sw/cmssw/blob/5622c7a49ec5a6d9234569e210b92efdc2f58cf7/RecoPixelVertexing/PixelLowPtUtilities/src/LowPtClusterShapeSeedComparitor.cc
Doesn't matter much anyway...
|
I would like the issue I raised being answered. Then the fix does not
necessarily need to fit in this PR, which by the way is already ok as
such for reco. Let wait for Monday: if Felice does not show up I'll
sign the PR and I'll open an issue for it.
Andrea Bocci <notifications@github.com> ha scritto:
… @perrotta @slava77 just to lrarify - do you want us to make any
changes, or are you OK with this as it is ?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#24288 (comment)
|
|
+1
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
Clean up some non-GPU-related pixel tracking code: clean up whitespaces and indenttion, plugin definitions, includes, and file names.
Add a customize function to provide a minimal pixel-tracking only configuration for profiling, removing validation, DQM, and output modules. Can be included in the configuration with
Backport from the Patatrack fork: cms-patatrack#49, cms-patatrack#122, cms-patatrack#134.