Fix ObjectID data type conversions in FarDetectorTrackerCluster #1477
Conversation
Capybara summary for PR 1477 |
wdconinc
changed the title
|
@simonge FYI, this fails to build. Do you think you might have a chance to look into this? |
| @@ -26,7 +26,7 @@ struct FDTrackerCluster { | |||
| double energy{0.0}; | |||
| double time{0.0}; | |||
| double timeError{0.0}; | |||
| std::vector<podio::ObjectID> rawHits; | |||
There was a problem hiding this comment.
Can you add a comment as to why we use edm4hep, not podio (which we use everywhere else) with a link to relevant definition? I would hate for someone to change it back to podio in a year from now.
Also, can this be considered a bug in podio or edm4hep or edm4eic? One would expect these all to have consistent ObjectID types.
There was a problem hiding this comment.
I couldn't quite work out how this was wrong here but the edm4hep::ObjectID type has already been removed from the recent edm4hep releases. This means if we update, the old edm4hep::TrackerHit I'm using will break it anyway as it contains a vector of objectIDs rather than using podio relations. I believe our data model has progressed to a state the work around isn't necessary anymore.
There was a problem hiding this comment.
What's still missing from edm4eic is a Measurement3D, effectively Measurement2D swapping surface for the extra dimension, through the transformation to global coordinates (or not transforming back to 2D).
|
Attempts to clean up the error:
/opt/local/include/edm4hep/ObjectID.h:26:71: runtime error: implicit conversion from type 'uint32_t' (aka 'unsigned int') of value 3167178110 (32-bit, unsigned) to type 'std::int32_t' (aka 'int') changed the value to -1127789186 (32-bit, signed)From -
#1 0x7ffb0e7d6ef4 in eicrecon::FarDetectorTrackerCluster::ConvertClusters(std::vector<FDTrackerCluster, std::allocator<FDTrackerCluster>> const&, edm4hep::TrackerHitCollection&) const /home/runner/work/EICrecon/EICrecon/src/algorithms/fardetectors/FarDetectorTrackerCluster.cc:198:29