Skip to content

Back to original try. No change in previous classes. And new v6 class. #4291

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 3 commits into from
Jun 9, 2026
Merged

Back to original try. No change in previous classes. And new v6 class. #4291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f71eb4
Back to original try. No change in previous classes. And new v6 class
Jun 8, 2026
badf5c7
use initializers in class, remove depracted methods - the base class …
pinkenburg Jun 9, 2026
95f0073
change NAN to numeric limits
osbornjd Jun 9, 2026
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
3 changes: 3 additions & 0 deletions offline/packages/trackbase/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ pkginclude_HEADERS = \
TrkrClusterv3.h \
TrkrClusterv4.h \
TrkrClusterv5.h \
TrkrClusterv6.h \
TrkrDefs.h \
TrkrHit.h \
TrkrHitSet.h \
Expand Down Expand Up @@ -187,6 +188,7 @@ ROOTDICTS = \
TrkrClusterv3_Dict.cc \
TrkrClusterv4_Dict.cc \
TrkrClusterv5_Dict.cc \
TrkrClusterv6_Dict.cc \
TrkrHitSetContMvtxHelper_Dict.cc \
TrkrHitSetContMvtxHelperv1_Dict.cc \
TrkrHitSetContainer_Dict.cc \
Expand Down Expand Up @@ -273,6 +275,7 @@ libtrack_io_la_SOURCES = \
TrkrClusterv3.cc \
TrkrClusterv4.cc \
TrkrClusterv5.cc \
TrkrClusterv6.cc \
TrkrDefs.cc \
TrkrHitSet.cc \
TrkrHitSetContMvtxHelper.cc \
Expand Down
54 changes: 39 additions & 15 deletions offline/packages/trackbase/TrkrCluster.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ class TrkrCluster : public PHObject
//
// cluster position
//
virtual float getLocalX() const { return NAN; }
virtual float getLocalX() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setLocalX(float) {}
virtual float getLocalY() const { return NAN; }
virtual float getLocalY() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setLocalY(float) {}

//
Expand All @@ -68,40 +68,64 @@ class TrkrCluster : public PHObject
virtual char getEdge() const { return std::numeric_limits<char>::max(); }
virtual void setEdge(char) {}
virtual void setTime(const float) {}
virtual float getTime() const { return NAN; }
virtual float getTime() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual char getSize() const { return std::numeric_limits<char>::max(); }

//
// convenience interface
//
virtual float getPhiSize() const { return NAN; }
virtual float getZSize() const { return NAN; }
virtual float getPhiError() const { return NAN; }
virtual float getRPhiError() const { return NAN; }
virtual float getZError() const { return NAN; }
virtual float getPhiSize() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getZSize() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getPhiError() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getRPhiError() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getZError() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual unsigned int getCenAdc() const { return UINT_MAX; }
virtual float getPadCen() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getTBinCen() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getPadMax() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getTBinMax() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual char getSLEdge() const { return std::numeric_limits<char>::max(); }
virtual char getSREdge() const { return std::numeric_limits<char>::max(); }
virtual char getTLEdge() const { return std::numeric_limits<char>::max(); }
virtual char getTREdge() const { return std::numeric_limits<char>::max(); }
virtual char getDLEdge() const { return std::numeric_limits<char>::max(); }
virtual char getDREdge() const { return std::numeric_limits<char>::max(); }
virtual char getHLEdge() const { return std::numeric_limits<char>::max(); }
virtual char getHREdge() const { return std::numeric_limits<char>::max(); }
virtual int getSLMix() const { return std::numeric_limits<int>::max(); }
virtual int getSRMix() const { return std::numeric_limits<int>::max(); }
virtual int getTLMix() const { return std::numeric_limits<int>::max(); }
virtual int getTRMix() const { return std::numeric_limits<int>::max(); }
virtual float getPhiBinLo() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getPhiBinHi() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getTBinLo() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getTBinHi() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getPadPhase() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getTBinPhase() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual float getRSize() const { return std::numeric_limits<float>::quiet_NaN(); }

/// Acts functions, for Acts modules use only
virtual void setActsLocalError(unsigned int /*i*/, unsigned int /*j*/, float /*value*/) {}
virtual float getActsLocalError(unsigned int /*i*/, unsigned int /*j*/) const { return NAN; }
virtual float getActsLocalError(unsigned int /*i*/, unsigned int /*j*/) const { return std::numeric_limits<float>::quiet_NaN(); }
virtual TrkrDefs::subsurfkey getSubSurfKey() const { return TrkrDefs::SUBSURFKEYMAX; }
virtual void setSubSurfKey(TrkrDefs::subsurfkey /*id*/) {}

// Global coordinate functions are deprecated, use local
// coordinate functions only
virtual float getX() const { return NAN; }
virtual float getX() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setX(float) {}
virtual float getY() const { return NAN; }
virtual float getY() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setY(float) {}
virtual float getZ() const { return NAN; }
virtual float getZ() const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setZ(float) {}
virtual float getPosition(int /*coor*/) const { return NAN; }
virtual float getPosition(int /*coor*/) const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setPosition(int /*coor*/, float /*xi*/) {}
virtual void setGlobal() {}
virtual void setLocal() {}
virtual bool isGlobal() const { return true; }
virtual float getError(unsigned int /*i*/, unsigned int /*j*/) const { return NAN; }
virtual float getError(unsigned int /*i*/, unsigned int /*j*/) const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setError(unsigned int /*i*/, unsigned int /*j*/, float /*value*/) {}
virtual float getSize(unsigned int /*i*/, unsigned int /*j*/) const { return NAN; }
virtual float getSize(unsigned int /*i*/, unsigned int /*j*/) const { return std::numeric_limits<float>::quiet_NaN(); }
virtual void setSize(unsigned int /*i*/, unsigned int /*j*/, float /*value*/) {}

protected:
Expand Down
100 changes: 100 additions & 0 deletions offline/packages/trackbase/TrkrClusterv6.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* @file trackbase/TrkrClusterv6.cc
* @author Ishan Goel
* @date May 2026
* @brief Implementation of TrkrClusterv6
*/
#include "TrkrClusterv6.h"

#include <cmath>
#include <utility> // for swap

namespace
{
// square convenience function
template <class T>
constexpr T square(const T& x)
{
return x * x;
}
} // namespace

void TrkrClusterv6::identify(std::ostream& os) const
{
os << "---TrkrClusterv6--------------------" << std::endl;

os << " (rphi,z) = (" << getLocalX();
os << ", " << getLocalY() << ") cm ";

os << " valid = " << isValid() << std::endl;

os << std::endl;
os << "-----------------------------------------------" << std::endl;

return;
}

int TrkrClusterv6::isValid() const
{
for (int i = 0; i < 2; ++i)
{
if (std::isnan(getPosition(i)))
{
return 0;
}
}
if (m_adc == 0xFFFF)
{
return 0;
}

return 1;
}

void TrkrClusterv6::CopyFrom(const TrkrCluster& source)
{
// do nothing if copying onto oneself
if (this == &source)
{
return;
}

// parent class method
TrkrCluster::CopyFrom(source);

setLocalX(source.getLocalX());
setLocalY(source.getLocalY());
setSubSurfKey(source.getSubSurfKey());
setAdc(source.getAdc());
setMaxAdc(source.getMaxAdc());
setCenAdc(source.getCenAdc());
setPadCen(source.getPadCen());
setTBinCen(source.getTBinCen());
setPadMax(source.getPadMax());
setTBinMax(source.getTBinMax());
setPhiError(source.getRPhiError());
setZError(source.getZError());
setRSize(source.getRSize());
setPhiSize(source.getPhiSize());
setZSize(source.getZSize());
Comment on lines +77 to +79

@coderabbitai coderabbitai Bot Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify sentinel-producing base getters and narrowing assignments in CopyFrom
rg -n -C2 'virtual (float getRSize|float getPhiSize|float getZSize|int getSLMix|int getSRMix|int getTLMix|int getTRMix)' offline/packages/trackbase/TrkrCluster.h
rg -n -C2 'set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\(' offline/packages/trackbase/TrkrClusterv6.cc

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 1706

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect TrkrClusterv6.cc around the reported CopyFrom assignments (and any nearby mix/size members)
sed -n '90,170p' offline/packages/trackbase/TrkrClusterv6.cc

# Inspect TrkrCluster.h around the sentinel defaults for the base getters
sed -n '60,140p' offline/packages/trackbase/TrkrCluster.h

# Confirm the exact setter signatures/types used by TrkrClusterv6 (so we know what conversions happen)
rg -n "void set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\(" offline/packages/trackbase/TrkrClusterv6.cc

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 5306

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Line numbers for CopyFrom assignments in TrkrClusterv6.cc
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\(source\.get" offline/packages/trackbase/TrkrClusterv6.cc

# Setter signatures/definitions in TrkrClusterv6.cc (parameter types)
rg -n "void set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\s*\(" offline/packages/trackbase/TrkrClusterv6.cc

# If setters are inline or inherited, also search for "setRSize" declarations
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\s*[,)]" offline/packages/trackbase/TrkrClusterv6.cc | head -n 50

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 327

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where setters are declared/overridden in TrkrClusterv6.cc (or any directly-included header within same file scope)
rg -n "setRSize|setPhiSize|setZSize|setSLMix|setSRMix|setTLMix|setTRMix" offline/packages/trackbase/TrkrClusterv6.cc

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 327

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate setter signatures in TrkrCluster.h (or others) to confirm conversion targets/types
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\s*\(" offline/packages/trackbase/TrkrCluster.h

# If not present there, search within trackbase for declarations
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\s*\(" offline/packages/trackbase | head -n 50

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 60

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Broader search for setter declarations/definitions (type-agnostic)
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)\s*\(" offline/packages/trackbase | head -n 200

# Also search for plain names without "(" in case of different syntax (e.g., member functions declared in classes)
rg -n "set(RSize|PhiSize|ZSize|SLMix|SRMix|TLMix|TRMix)" offline/packages/trackbase | head -n 200

Repository: sPHENIX-Collaboration/coresoftware

Length of output: 4010

Guard sentinel-to-narrow assignments in TrkrClusterv6::CopyFrom to prevent corrupted cluster state.

TrkrCluster base getters return NAN for getRSize/getPhiSize/getZSize and std::numeric_limits<int>::max() for getSLMix/getSRMix/getTLMix/getTRMix. TrkrClusterv6::CopyFrom then writes these directly into narrow types (setRSize(unsigned char), setPhiSize(char), setZSize(char), and set*Mix(char)) at lines 118-120 and 131-134, so NAN→integer conversion is undefined and INT_MAX→char produces invalid/corrupted values for non-v6 sources.

Add sentinel/range checks (or only copy these fields when the source is known-valid for v6) before calling the narrow setters.

setOverlap(source.getOverlap());
setEdge(source.getEdge());
setSLEdge(source.getSLEdge());
setSREdge(source.getSREdge());
setTLEdge(source.getTLEdge());
setTREdge(source.getTREdge());
setDLEdge(source.getDLEdge());
setDREdge(source.getDREdge());
setHLEdge(source.getHLEdge());
setHREdge(source.getHREdge());
setSLMix(source.getSLMix());
setSRMix(source.getSRMix());
setTLMix(source.getTLMix());
setTRMix(source.getTRMix());
setPhiBinLo(source.getPhiBinLo());
setPhiBinHi(source.getPhiBinHi());
setTBinLo(source.getTBinLo());
setTBinHi(source.getTBinHi());
setPadPhase(source.getPadPhase());
setTBinPhase(source.getTBinPhase());
}
Loading