Skip to content

New setters for lamination fitting code #4270

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
pinkenburg merged 1 commit into from
May 19, 2026
Merged

New setters for lamination fitting code #4270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 19 additions & 6 deletions offline/packages/tpccalib/TpcLaminationFitting.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ void TpcLaminationFitting::set_grid_dimensions(int phibins, int rbins)
m_rbins = rbins;
}

//___________________________________________________________
void TpcLaminationFitting::set_lam_grid_dimensions(int phibins, int rbins)
{
m_lamPhiBins = phibins;
m_lamRBins = rbins;
}
Comment on lines +61 to +65
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate lamination grid dimensions before storing them.

Line 61 accepts unchecked bin counts, and Lines 101/109 use them directly in TH2D allocation. Non-positive or extreme values can create invalid histograms or trigger large memory spikes.

Proposed fix 
 void TpcLaminationFitting::set_lam_grid_dimensions(int phibins, int rbins)
 {
-  m_lamPhiBins = phibins;
-  m_lamRBins = rbins;
+  constexpr int kMinBins = 1;
+  constexpr int kMaxBins = 2000;
+  if (phibins < kMinBins || rbins < kMinBins || phibins > kMaxBins || rbins > kMaxBins)
+  {
+    std::cout << PHWHERE
+              << " invalid lamination grid dimensions (phi=" << phibins
+              << ", r=" << rbins << "), keeping previous values (phi="
+              << m_lamPhiBins << ", r=" << m_lamRBins << ")"
+              << std::endl;
+    return;
+  }
+  m_lamPhiBins = phibins;
+  m_lamRBins = rbins;
 }

As per coding guidelines, **/*.{cc,cpp,cxx,c} files should prioritize correctness and error handling.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void TpcLaminationFitting::set_lam_grid_dimensions(int phibins, int rbins)
{
m_lamPhiBins = phibins;
m_lamRBins = rbins;
}
void TpcLaminationFitting::set_lam_grid_dimensions(int phibins, int rbins)
{
constexpr int kMinBins = 1;
constexpr int kMaxBins = 2000;
if (phibins < kMinBins || rbins < kMinBins || phibins > kMaxBins || rbins > kMaxBins)
{
std::cout << PHWHERE
<< " invalid lamination grid dimensions (phi=" << phibins
<< ", r=" << rbins << "), keeping previous values (phi="
<< m_lamPhiBins << ", r=" << m_lamRBins << ")"
<< std::endl;
return;
}
m_lamPhiBins = phibins;
m_lamRBins = rbins;
}


//____________________________________________
int TpcLaminationFitting::InitRun(PHCompositeNode *topNode)
{
Expand Down Expand Up @@ -91,15 +98,15 @@ int TpcLaminationFitting::InitRun(PHCompositeNode *topNode)

if(m_fieldOff)
{
m_hLamination[l][s] = new TH2D((boost::format("hLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), (boost::format("Lamination %d %s, #phi_{ideal}=%.2f;R [cm];#phi") %l %(s == 1 ? "North" : "South") %m_laminationIdeal[l][s]).str().c_str(), 200, 30, 80, 200, m_laminationIdeal[l][s] - 0.2, m_laminationIdeal[l][s] + 0.2);
m_hLamination[l][s] = new TH2D((boost::format("hLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), (boost::format("Lamination %d %s, #phi_{ideal}=%.2f;R [cm];#phi") %l %(s == 1 ? "North" : "South") %m_laminationIdeal[l][s]).str().c_str(), m_lamRBins, 30, 80, m_lamPhiBins, m_laminationIdeal[l][s] - 0.2, m_laminationIdeal[l][s] + 0.2);
m_fLamination[l][s] = new TF1((boost::format("fLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), "[0]+[1]", 30, 80);
m_fLamination[l][s]->SetParameters(m_laminationOffset[l][s], m_laminationIdeal[l][s]);
m_fLamination[l][s]->SetParLimits(0, -0.05, 0.05);
m_fLamination[l][s]->FixParameter(1, m_laminationIdeal[l][s]);
}
else
{
m_hLamination[l][s] = new TH2D((boost::format("hLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), (boost::format("Lamination %d %s, #phi_{nominal}=%.2f;R [cm];#phi") %l %(s == 1 ? "North" : "South") %(m_laminationIdeal[l][s]+m_laminationOffset[l][s])).str().c_str(), 200, 30, 80, 200, m_laminationIdeal[l][s]+m_laminationOffset[l][s] - 0.2, m_laminationIdeal[l][s]+m_laminationOffset[l][s] + 0.2);
m_hLamination[l][s] = new TH2D((boost::format("hLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), (boost::format("Lamination %d %s, #phi_{nominal}=%.2f;R [cm];#phi") %l %(s == 1 ? "North" : "South") %(m_laminationIdeal[l][s]+m_laminationOffset[l][s])).str().c_str(), m_lamRBins, 30, 80, m_lamPhiBins, m_laminationIdeal[l][s]+m_laminationOffset[l][s] - 0.2, m_laminationIdeal[l][s]+m_laminationOffset[l][s] + 0.2);
m_fLamination[l][s] = new TF1((boost::format("fLamination%d_%s") %l %(s == 1 ? "North" : "South")).str().c_str(), "[3]+[0]*(1-exp(-[2]*(x-[1])))", 30, 80);
m_fLamination[l][s]->SetParameters(-0.08, 38, 0.16, 0.0);
m_fLamination[l][s]->SetParLimits(0, -0.02, 0.0);
Expand Down Expand Up @@ -394,6 +401,12 @@ int TpcLaminationFitting::process_event(PHCompositeNode *topNode)
continue;
}

double weight = 1.0;
if(m_adcWeight)
{
weight = 1.0*cmclus->getAdc();
}


//Acts::Vector3 pos(cmclus->getX(), cmclus->getY(), cmclus->getZ());
Acts::Vector3 pos(cmclus->getX(), cmclus->getY(), (side ? 1.0 : -1.0));
Expand All @@ -408,7 +421,7 @@ int TpcLaminationFitting::process_event(PHCompositeNode *topNode)

TVector3 tmp_pos(pos[0], pos[1], pos[2]);

if(cmclus->getNLayers() > m_nLayerCut && cmclus->getSDWeightedLayer() > 0.5)
if(cmclus->getNLayers() > m_nLayerCut && (!m_useSDLayerCut || cmclus->getSDWeightedLayer() > 0.5))
{
for (int l = 0; l < 18; l++)
{
Expand All @@ -426,12 +439,12 @@ int TpcLaminationFitting::process_event(PHCompositeNode *topNode)

if (phi2pi > shift - 0.2 && phi2pi < shift + 0.2)
{
m_hLamination[l][side]->Fill(tmp_pos.Perp(), phi2pi);
m_hLamination[l][side]->Fill(tmp_pos.Perp(), phi2pi, weight);
}
}
}

if(cmclus->getSDWeightedLayer() > 0.5)
if(m_useSDLayerCut && cmclus->getSDWeightedLayer() > 0.5)
{
continue;
}
Expand All @@ -450,7 +463,7 @@ int TpcLaminationFitting::process_event(PHCompositeNode *topNode)
phi2pimod -= M_PI / 9;
}

m_hPetal[side]->Fill(phi2pimod, tmp_pos.Perp());
m_hPetal[side]->Fill(phi2pimod, tmp_pos.Perp(), weight);
}

return Fun4AllReturnCodes::EVENT_OK;
Expand Down
11 changes: 11 additions & 0 deletions offline/packages/tpccalib/TpcLaminationFitting.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ class TpcLaminationFitting : public SubsysReco

void set_nLayerCut(unsigned int cut) { m_nLayerCut = cut; }

void set_useSDLayerCut(bool useCut) { m_useSDLayerCut = useCut; }

void set_adcWeight(bool useADC) { m_adcWeight = useADC; }

void set_lam_grid_dimensions(int phibins, int rbins);

int InitRun(PHCompositeNode *topNode) override;

int process_event(PHCompositeNode *topNode) override;
Expand Down Expand Up @@ -110,6 +116,8 @@ class TpcLaminationFitting : public SubsysReco
//TH2 *scaleFactorMap[2]{nullptr};

unsigned int m_nLayerCut{1};
bool m_useSDLayerCut{true};
bool m_adcWeight{false};

bool m_useHeader{true};

Expand Down Expand Up @@ -148,6 +156,9 @@ class TpcLaminationFitting : public SubsysReco
double m_rmse{};
int m_nBins{0};

int m_lamPhiBins{200};
int m_lamRBins{200};

int m_phibins{80};
static constexpr float m_phiMin{0};
static constexpr float m_phiMax{2. * M_PI};
Expand Down