added initialization to CRTHit data members #161
Conversation
mt82
changed the title
kjplows
left a comment
kjplows
left a comment
There was a problem hiding this comment.
Looks good to me thanks! I'll let @PetrilloAtWork decide if using flavours of 0 is better than calling on std::numeric_limits<T> here; my one comment is if any CRT variables are expected to be 0 physically (looking at ts0_ns, x_pos etc) probably std::numeric_limits::lowest() is easier to interpret as nonsense.
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
…: plain -1 for integer-like variables and std::numeric_limits<>::lowest() for the others
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Approved, with my thanks.
|
trigger build ci_ref=v10_14_00_01 LArSoft/larpandoracontent@LARSOFT_SUITE_v10_14_02_01 LArSoft/larpandora@LARSOFT_SUITE_v10_14_02_01 LArSoft/larreco@LARSOFT_SUITE_v10_14_02_01 LArSoft/larrecodnn@LARSOFT_SUITE_v10_14_02_01 LArSoft/larana@LARSOFT_SUITE_v10_14_02_01 LArSoft/lareventdisplay@LARSOFT_SUITE_v10_14_02_01 LArSoft/larsoft@LARSOFT_SUITE_v10_14_02_01 LArSoft/lar*@LARSOFT_SUITE_v10_14_02 SBNSoftware/sbndaq-artdaq-core@v1_10_06 SBNSoftware/sbn*@SBN_SUITE_v10_14_02 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_02 SBNSoftware/sbncode@v10_14_02 SBNSoftware/sbn*@SBN_SUITE_v10_14_02 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_02_01 SBNSoftware/sbncode@v10_14_02_01 SBNSoftware/sbn*@SBN_SUITE_v10_14_02_01 |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
sbnobj/Common/CRT/CRTHit.hh
Outdated
| double ts0_s_corr; ///< [Honestly, not sure at this point, it was there since long time (BB)] | ||
| double ts0_ns; ///< Timestamp T0 (from White Rabbit), in UTC absolute time scale in nanoseconds from the Epoch. | ||
| double ts0_ns_corr; ///< [Honestly, not sure at this point, it was there since long time (BB)] | ||
| uint64_t ts0_s{-1}; ///< Second-only part of timestamp T0. |
There was a problem hiding this comment.
Noting this error:
4348: /scratch/workspace/sbnd_ci/label_exp/swarm/label_exp2/swarm/SBND/srcs/sbnobj/sbnobj/Common/CRT/CRTHit.hh:32:28: error: constant expression evaluates to -1 which cannot be narrowed to type 'uint64_t' (aka 'unsigned long') [-Wc++11-narrowing]
4349: uint64_t ts0_s{-1}; ///< Second-only part of timestamp T0.
4350: ^~
4351: /scratch/workspace/sbnd_ci/label_exp/swarm/label_exp2/swarm/SBND/srcs/sbnobj/sbnobj/Common/CRT/CRTHit.hh:32:28: note: insert an explicit cast to silence this issue
4352: uint64_t ts0_s{-1}; ///< Second-only part of timestamp T0.
4353: ^~
4354: static_cast<uint64_t>( )
Inserting static_cast to fix this issue. Note that the actual value becomes 2^64 - 1 = 18446744073709551615.
|
trigger build LArSoft/lar*@LARSOFT_SUITE_v10_14_02_01 SBNSoftware/sbncode@v10_14_02_01 SBNSoftware/sbn*@SBN_SUITE_v10_14_02_01 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests SBND phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs parent CI build details are available through the CI dashboard |
Default initialization values have been introduced for all CRTHit data members. This change ensures that variables are properly initialized at object creation, reducing the risk of undefined behavior and improving code reliability.