Skip to content

Commit 67ab2c1

Browse files
committed
Code Review
Address Major Issues: - Ensure m_directURL_EventPlaneCalib is not empty before passing it to hasValidTree - Removed unused vectors in EventPlaneRecov2 (south_psi, north_psi, northsouth_psi) - Guard against order == 0 to avoid divide-by-zero. - Add bounds checks before indexing Q-vector storage.
1 parent 1e20617 commit 67ab2c1

File tree

3 files changed

+46
-19
lines changed

3 files changed

+46
-19
lines changed

offline/packages/eventplaneinfo/EventPlaneRecov2.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ int EventPlaneRecov2::Init(PHCompositeNode *topNode)
7878
{
7979
std::string calibdir = CDBInterface::instance()->getUrl(m_calibName);
8080

81-
if (hasValidTree(m_directURL_EventPlaneCalib))
81+
if (!m_directURL_EventPlaneCalib.empty() && hasValidTree(m_directURL_EventPlaneCalib))
8282
{
8383
m_cdbttree = std::make_unique<CDBTTree>(m_directURL_EventPlaneCalib);
8484
std::cout << PHWHERE << " Custom Event Plane Calib Found: " << m_directURL_EventPlaneCalib << std::endl;
@@ -552,10 +552,6 @@ int EventPlaneRecov2::FillNode(PHCompositeNode *topNode)
552552
std::vector<std::pair<double, double>> northsouth_Qvec_recentered(vec_size, {NAN, NAN});
553553
std::vector<std::pair<double, double>> northsouth_Qvec(vec_size, {NAN, NAN});
554554

555-
std::vector<double> south_psi(vec_size, NAN);
556-
std::vector<double> north_psi(vec_size, NAN);
557-
std::vector<double> northsouth_psi(vec_size, NAN);
558-
559555
for (size_t h_idx = 0; h_idx < m_harmonics.size(); ++h_idx)
560556
{
561557
int n = m_harmonics[h_idx];

offline/packages/eventplaneinfo/Eventplaneinfov2.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ void Eventplaneinfov2::identify(std::ostream& os) const
1010

1111
double Eventplaneinfov2::GetPsi(const double Qx, const double Qy, const unsigned int order) const
1212
{
13-
double temp;
14-
if ((Qx == 0.0) && (Qy == 0.0))
13+
if (order == 0)
1514
{
16-
temp = NAN;
15+
return NAN;
1716
}
18-
else
17+
if ((Qx == 0.0) && (Qy == 0.0))
1918
{
20-
temp = atan2(Qy, Qx) / ((double) order);
19+
return NAN;
2120
}
22-
return temp;
21+
return atan2(Qy, Qx) / static_cast<double>(order);
2322
}

offline/packages/eventplaneinfo/Eventplaneinfov2.h

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,49 @@ class Eventplaneinfov2 : public Eventplaneinfo
3232
void set_qvector_raw(const std::vector<std::pair<double, double>>& Qvec) override { mQvec_raw = Qvec; }
3333
void set_qvector_recentered(const std::vector<std::pair<double, double>>& Qvec) override { mQvec_recentered = Qvec; }
3434
void set_shifted_psi(std::vector<double> Psi_Shifted) override { mPsi_Shifted = Psi_Shifted; }
35-
std::pair<double, double> get_qvector(int order) const override { return std::make_pair(mQvec[order - 1].first, mQvec[order - 1].second); }
36-
std::pair<double, double> get_qvector_raw(int order) const override { return std::make_pair(mQvec_raw[order - 1].first, mQvec_raw[order - 1].second); }
37-
std::pair<double, double> get_qvector_recentered(int order) const override { return std::make_pair(mQvec_recentered[order - 1].first, mQvec_recentered[order - 1].second); }
35+
std::pair<double, double> get_qvector(int order) const override { return safe_qvec(mQvec, order); }
36+
std::pair<double, double> get_qvector_raw(int order) const override { return safe_qvec(mQvec_raw, order); }
37+
std::pair<double, double> get_qvector_recentered(int order) const override { return safe_qvec(mQvec_recentered, order); }
3838
void set_ring_qvector(std::vector<std::vector<std::pair<double, double>>> Qvec) override { ring_Qvec = Qvec; }
39-
std::pair<double, double> get_ring_qvector(int ring_index, int order) const override { return ring_Qvec[ring_index][order - 1]; }
40-
double get_ring_psi(int ring_index, int order) const override {return GetPsi(ring_Qvec[ring_index][order - 1].first,ring_Qvec[ring_index][order - 1].second,order);}
39+
std::pair<double, double> get_ring_qvector(int ring_index, int order) const override
40+
{
41+
if (ring_index < 0 || static_cast<size_t>(ring_index) >= ring_Qvec.size())
42+
{
43+
return {NAN, NAN};
44+
}
45+
return safe_qvec(ring_Qvec[ring_index], order);
46+
}
47+
double get_ring_psi(int ring_index, int order) const override
48+
{
49+
auto q = get_ring_qvector(ring_index, order);
50+
return GetPsi(q.first, q.second, static_cast<unsigned int>(order));
51+
}
52+
4153
double GetPsi(double Qx, double Qy, unsigned int order) const override;
42-
double get_psi(int order) const override { return GetPsi(mQvec[order - 1].first, mQvec[order - 1].second, order);}
43-
double get_shifted_psi(int order) const override { return mPsi_Shifted[order - 1]; }
44-
54+
double get_psi(int order) const override
55+
{
56+
auto q = get_qvector(order);
57+
return GetPsi(q.first, q.second, static_cast<unsigned int>(order));
58+
}
59+
double get_shifted_psi(int order) const override
60+
{
61+
if (order <= 0 || static_cast<size_t>(order) > mPsi_Shifted.size())
62+
{
63+
return NAN;
64+
}
65+
return mPsi_Shifted[order - 1];
66+
}
67+
4568
private:
69+
static std::pair<double, double> safe_qvec(const std::vector<std::pair<double, double>>& v, int order)
70+
{
71+
if (order <= 0 || static_cast<size_t>(order) > v.size())
72+
{
73+
return {NAN, NAN};
74+
}
75+
return v[order - 1];
76+
}
77+
4678
std::vector<std::pair<double, double>> mQvec;
4779
std::vector<std::pair<double, double>> mQvec_raw;
4880
std::vector<std::pair<double, double>> mQvec_recentered;

0 commit comments

Comments
 (0)