Skip to content

Disable computation of Q probability in SiPixel template CPE for iterative tracking #39776

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.

Sign up for GitHub

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

Merged
cmsbuild merged 5 commits into from
Oct 28, 2022
Merged

Disable computation of Q probability in SiPixel template CPE for iterative tracking #39776

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f6282af
Use SiPixel CPE (cluster repair) with speed=0; i.e., without ProbQ co…
mmasciov Oct 17, 2022
5aa1f4a
Remove warning when disabling probQ computation
mmasciov Oct 17, 2022
2bde976
update WithoutProbQ for phase2
slava77 Oct 19, 2022
8b4f5df
Restore previous warning, except for default value
mmasciov Oct 22, 2022
bd6dab7
Use float representation in comparison
mmasciov Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DataFormats/TrackerRecHit2D/interface/SiPixelRecHitQuality.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class SiPixelRecHitQuality {
if (prob > 1.0f && prob <= 1.0f + std::numeric_limits<float>::epsilon()) {
prob = 1;
} else if (prob < 0.0f || prob > 1.0f + std::numeric_limits<float>::epsilon()) {
warningOutOfBoundProb("Q", prob, qualWord);
//warningOutOfBoundProb("Q", prob, qualWord);
Copy link
Contributor

@mmusich mmusich Oct 20, 2022

Choose a reason for hiding this comment

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

As per previous statement (cf #38967 (comment)) commented code is not advised in the reco stack.
If you are going to keep it, then at least put a comment on why it is so.

Copy link
Contributor Author

@mmasciov mmasciov Oct 20, 2022

Choose a reason for hiding this comment

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

If there's no objection, I can simply delete this line. I kept it commented out in case others think it may be useful to keep. If not, I will delete it.

Copy link
Contributor

@slava77 slava77 Oct 20, 2022

Choose a reason for hiding this comment

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

I would guess that the default value should not get a warning, but perhaps everything else still should.
@cms-sw/trk-dpg-l2 please advise

Copy link
Contributor

@slava77 slava77 Oct 21, 2022

Choose a reason for hiding this comment

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

I would guess that the default value should not get a warning, but perhaps everything else still should. @cms-sw/trk-dpg-l2 please advise

I guess my tag went only to the DPG conveners
@ferencek @mroguljic
(please clarify also if there is a reco contact replacing Oz)

Copy link
Contributor

@ferencek ferencek Oct 21, 2022

Choose a reason for hiding this comment

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

We've been looking (unsuccessfully) for a reco contact for more than a year now.

Regarding the proposed code changes, simply commenting out or deleting a call to warningOutOfBoundProb does not look like a good solution to me as it changes the legacy behavior of this code. Possible solutions:

  1. As Slava suggested, modify the code so that the default value does not cause a warning
  2. Add some sort of verbosity flag which would allow suppressing warnings when the Q probability is not computed.

Copy link
Contributor Author

@mmasciov mmasciov Oct 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@ferencek, this is addressed in 8b4f5df.
Only the default value does not cause the warning, which otherwise is still there and active.

prob = 0;
}
double draw = (prob <= 1E-5) ? 255 : -log((double)prob) * probY_1_over_log_units;
Expand Down
5 changes: 4 additions & 1 deletion RecoLocalTracker/SiPixelRecHits/python/PixelCPEClusterRepair_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@

from RecoLocalTracker.SiPixelRecHits._templates2_default_cfi import _templates2_default
templates2 = _templates2_default.clone()

templates2_speed0 = _templates2_default.clone(
ComponentName = "PixelCPEClusterRepairWithoutProbQ",
speed = 0
)
1 change: 1 addition & 0 deletions RecoTracker/TrackProducer/python/TrackProducerIterativeDefault_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
useSimpleMF = cms.bool(True),
SimpleMagneticField = cms.string("ParabolicMf"),
Propagator = cms.string('PropagatorWithMaterialParabolicMf'),
TTRHBuilder = cms.string('WithAngleAndTemplateWithoutProbQ')
)
5 changes: 4 additions & 1 deletion RecoTracker/TransientTrackingRecHit/python/TTRHBuilderWithTemplate_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
from Configuration.Eras.Modifier_phase1Pixel_cff import phase1Pixel
phase1Pixel.toModify(TTRHBuilderAngleAndTemplate, PixelCPE = 'PixelCPEClusterRepair')

TTRHBuilderAngleAndTemplateWithoutProbQ = TTRHBuilderAngleAndTemplate.clone(ComponentName = 'WithAngleAndTemplateWithoutProbQ')
phase1Pixel.toModify(TTRHBuilderAngleAndTemplateWithoutProbQ, PixelCPE = 'PixelCPEClusterRepairWithoutProbQ')

# Turn off template reco for phase 2 (when not supported)
from Configuration.ProcessModifiers.PixelCPEGeneric_cff import PixelCPEGeneric
PixelCPEGeneric.toModify(TTRHBuilderAngleAndTemplate, PixelCPE = 'PixelCPEGeneric')

PixelCPEGeneric.toModify(TTRHBuilderAngleAndTemplateWithoutProbQ, PixelCPE = 'PixelCPEGeneric')