-
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
Tracker phase2 bricked pixel localreco cmssw 12 1 x #35441
Changes from 8 commits
33f5fe3
4802506
feee05e
0c81bc7
52deeb5
03aec9a
603cd26
c25822b
281982a
ea7ba62
4c496f3
861a1b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -28,8 +28,6 @@ namespace SiPixelUtils { | |||||
float eff_charge_cut_high, //!< Use edge if < w_eff &&& | ||||||
float size_cut //!< Use edge when size == cuts | ||||||
) { | ||||||
//cout<<" in PixelCPEGeneric:generic_position_formula - "<<endl; //dk | ||||||
|
||||||
float geom_center = 0.5f * (upper_edge_first_pix + lower_edge_last_pix); | ||||||
|
||||||
//--- The case of only one pixel in this projection is separate. Note that | ||||||
|
@@ -47,8 +45,6 @@ namespace SiPixelUtils { | |||||
float w_pred = theThickness * cot_angle // geometric correction (in cm) | ||||||
- lorentz_shift; // (in cm) &&& check fpix! | ||||||
|
||||||
//cout << " in PixelCPEGeneric:generic_position_formula - " << w_inner << " " << w_pred << endl; // dk | ||||||
|
||||||
//--- Total length of the two edge pixels (first+last) | ||||||
float sum_of_edge = 2.0f; | ||||||
if (first_is_big) | ||||||
|
@@ -60,7 +56,7 @@ namespace SiPixelUtils { | |||||
float w_eff = std::abs(w_pred) - w_inner; | ||||||
|
||||||
//--- If the observed charge width is inconsistent with the expectations | ||||||
//--- based on the track, do *not* use w_pred-W_innner. Instead, replace | ||||||
//--- based on the track, do *not* use w_pred-w_innner. Instead, replace | ||||||
//--- it with an *average* effective charge width, which is the average | ||||||
//--- length of the edge pixels. | ||||||
// | ||||||
|
@@ -71,17 +67,84 @@ namespace SiPixelUtils { | |||||
} | ||||||
|
||||||
//--- Finally, compute the position in this projection | ||||||
float qdiff = q_l - q_f; | ||||||
float qsum = q_l + q_f; | ||||||
float q_diff = q_l - q_f; | ||||||
float q_sum = q_l + q_f; | ||||||
|
||||||
//--- Temporary fix for clusters with both first and last pixel with charge = 0 | ||||||
if (qsum == 0) | ||||||
qsum = 1.0f; | ||||||
if (q_sum == 0) | ||||||
q_sum = 1.0f; | ||||||
|
||||||
//float hit_pos = geom_center + 0.5f*(qdiff/qsum) * w_eff + half_lorentz_shift; | ||||||
float hit_pos = geom_center + 0.5f * (qdiff / qsum) * w_eff; | ||||||
float hit_pos = geom_center + 0.5f * (q_diff / q_sum) * w_eff; | ||||||
|
||||||
return hit_pos; | ||||||
} | ||||||
|
||||||
float generic_position_formula_y_bricked( | ||||||
int size, //!< Size of this projection. | ||||||
int q_f, //!< Charge in the first pixel. | ||||||
int q_l, //!< Charge in the last pixel. | ||||||
int q_f_b, //!< Charge in pixels that are "dented" compared to the lowest pixel of the cluster. | ||||||
int q_l_b, //!< Charge in pixels that are "dented" compared to the highest pixel of the cluster. | ||||||
float upper_edge_first_pix, //!< As the name says. | ||||||
float lower_edge_last_pix, //!< As the name says. | ||||||
float lorentz_shift, //!< L-shift at half thickness | ||||||
float theThickness, //detector thickness | ||||||
float cot_angle, //!< cot of alpha_ or beta_ | ||||||
float pitch, //!< thePitchX or thePitchY | ||||||
bool first_is_big, //!< true if the first is big | ||||||
bool last_is_big, //!< true if the last is big | ||||||
float eff_charge_cut_low, //!< Use edge if > w_eff &&& | ||||||
float eff_charge_cut_high, //!< Use edge if < w_eff &&& | ||||||
float size_cut //!< Use edge when size == cuts | ||||||
) { | ||||||
float geom_center = 0.5f * (upper_edge_first_pix + lower_edge_last_pix); | ||||||
|
||||||
//--- The case of only one pixel in this projection is separate. Note that | ||||||
//--- here first_pix == last_pix, so the average of the two is still the | ||||||
//--- center of the pixel. | ||||||
|
||||||
//--- Width of the clusters minus the edge (first and last) pixels. | ||||||
//--- In the note, they are denoted x_F and x_L (and y_F and y_L) | ||||||
float w_inner = lower_edge_last_pix - upper_edge_first_pix; // in cm | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
|
||||||
//--- Predicted charge width from geometry | ||||||
float w_pred = theThickness * cot_angle // geometric correction (in cm) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
- lorentz_shift; // (in cm) &&& check fpix! | ||||||
|
||||||
//--- Total length of the two edge pixels (first+last) | ||||||
float sum_of_edge = 2.0f; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
if (first_is_big) | ||||||
sum_of_edge += 1.0f; | ||||||
if (last_is_big) | ||||||
sum_of_edge += 1.0f; | ||||||
|
||||||
//--- The `effective' charge width -- particle's path in first and last pixels only | ||||||
float w_eff = std::abs(w_pred) - std::abs(w_inner); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
|
||||||
//--- If the observed charge width is inconsistent with the expectations | ||||||
//--- based on the track, do *not* use w_pred-w_innner. Instead, replace | ||||||
//--- it with an *average* effective charge width, which is the average | ||||||
//--- length of the edge pixels. | ||||||
// | ||||||
// bool usedEdgeAlgo = false; | ||||||
//Modified cut to make use of the w_eff in the bricked geometry | ||||||
if (size >= size_cut) { | ||||||
w_eff = pitch * 0.5f * sum_of_edge; // ave. length of edge pixels (first+last) (cm) | ||||||
// usedEdgeAlgo = true; | ||||||
} | ||||||
|
||||||
//--- Finally, compute the position in this projection | ||||||
float q_diff = q_l - q_f; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
float q_sum = q_l + q_f; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
float q_b_corr = q_l_b + q_f_b; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||||||
|
||||||
//--- Temporary fix for clusters with both first and last pixel with charge = 0 | ||||||
if (q_sum == 0) | ||||||
q_sum = 1.0f; | ||||||
|
||||||
float hit_pos = | ||||||
geom_center + 0.5f * (q_diff / q_sum) * w_eff + 0.5f * (q_b_corr / q_sum) * w_eff; //bricked correction | ||||||
|
||||||
return hit_pos; | ||||||
} | ||||||
} // namespace SiPixelUtils |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import FWCore.ParameterSet.Config as cms | ||
|
||
from Configuration.Eras.Era_Phase2C11I13_cff import Phase2C11I13 | ||
from Configuration.Eras.Modifier_phase2_3DPixels_cff import phase2_3DPixels | ||
from Configuration.Eras.Modifier_phase2_squarePixels_cff import phase2_squarePixels | ||
from Configuration.Eras.Modifier_phase2_brickedPixels_cff import phase2_brickedPixels | ||
from Configuration.Eras.Modifier_phase2_GE0_cff import phase2_GE0 | ||
|
||
Phase2C11I13T27M9 = cms.ModifierChain(Phase2C11I13, phase2_3DPixels, phase2_squarePixels, phase2_brickedPixels, phase2_GE0) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
import FWCore.ParameterSet.Config as cms | ||
|
||
phase2_brickedPixels = cms.Modifier() |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,4 @@ | ||||||
import FWCore.ParameterSet.Config as cms | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not entirely sure I understand the case for this modifier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea was to have the modifier phase2_brickedPixels devoted to the changes to the clusterizer and to the CPE generic algo needed for the bricked topology and a different modifier PixeCPEGenericForBricked to turn off template reco as we do in case of T22/2026D78 geometry. e.g, PixelCPEGenericForBricked is (supposed to be) effective only in track reco while phase2_brickedPixels is (supposed to be) effective already in local reco There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but at the moment you cannot even envisage a bricked pixels wf that doesn't run generic, isn't it? cmssw/RecoTracker/TransientTrackingRecHit/python/TTRHBuilderWithTemplate_cfi.py Lines 22 to 23 in c25822b
and remove the modification. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was corrected in the last commit by doing what you suggested @mmusich |
||||||
|
||||||
# This modifier is to run the Pixel Generic CPE algorithm specialized for bricked pixels in Phase-2 workflows | ||||||
PixelCPEGenericForBricked = cms.Modifier() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convention is to start with lower case letter
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it was coded uppercase for consistency with Should we change both? |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,9 +102,8 @@ bool PixelThresholdClusterizer::setup(const PixelGeomDetUnit* pixDet) { | |
theNumOfCols = ncols; | ||
|
||
if (nrows > theBuffer.rows() || ncols > theBuffer.columns()) { // change only when a larger is needed | ||
//if( nrows != theNumOfRows || ncols != theNumOfCols ) { | ||
//cout << " PixelThresholdClusterizer: pixel buffer redefined to " | ||
// << nrows << " * " << ncols << endl; | ||
if (nrows != theNumOfRows || ncols != theNumOfCols) | ||
edm::LogWarning("setup()") << "pixel buffer redefined to" << nrows << " * " << ncols; | ||
//theNumOfRows = nrows; // Set new sizes | ||
//theNumOfCols = ncols; | ||
// Resize the buffer | ||
|
@@ -132,7 +131,8 @@ void PixelThresholdClusterizer::clusterizeDetUnitT(const T& input, | |
typename T::const_iterator end = input.end(); | ||
|
||
// Do not bother for empty detectors | ||
//if (begin == end) cout << " PixelThresholdClusterizer::clusterizeDetUnit - No digis to clusterize"; | ||
if (begin == end) | ||
edm::LogWarning("clusterizeDetUnit()") << "No digis to clusterize"; | ||
|
||
// Set up the clusterization on this DetId. | ||
if (!setup(pixDet)) | ||
|
@@ -152,7 +152,7 @@ void PixelThresholdClusterizer::clusterizeDetUnitT(const T& input, | |
|
||
assert(output.empty()); | ||
// Loop over all seeds. TO DO: wouldn't using iterators be faster? | ||
// edm::LogError("PixelThresholdClusterizer") << "Starting clusterizing" << endl; | ||
// LogError("PixelThresholdClusterizer") << "Starting clusterizing"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this actually be removed, or converted to LogDebug? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it! |
||
for (unsigned int i = 0; i < theSeeds.size(); i++) { | ||
// Gavril : The charge of seeds that were already inlcuded in clusters is set to 1 electron | ||
// so we don't want to call "make_cluster" for these cases | ||
|
@@ -163,7 +163,7 @@ void PixelThresholdClusterizer::clusterizeDetUnitT(const T& input, | |
// Check if the cluster is above threshold | ||
// (TO DO: one is signed, other unsigned, gcc warns...) | ||
if (cluster.charge() >= clusterThreshold) { | ||
// std::cout << "putting in this cluster " << i << " " << cluster.charge() << " " << cluster.pixelADC().size() << endl; | ||
// LogDebug("clusterizeDetUnit():") << "putting in this cluster" << i << cluster.charge() << cluster.pixelADC().size(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this actually be removed? Or uncommented? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it |
||
// sort by row (x) | ||
output.push_back(std::move(cluster)); | ||
std::push_heap(output.begin(), output.end(), [](SiPixelCluster const& cl1, SiPixelCluster const& cl2) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed