Draft: Start to migrate the R4-MS segment fit #4108
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughNew headers and include directives, added they have been. In StationSpacePoint.hpp, a concept defined is; in MuonSpacePoint.hpp, a class for muon measurements now exists. Additional include directives enable access to the new muon space point definitions in existing modules, and a CSV structure for muon space point data is introduced. Changes, all related to muon measurement handling in composite detectors, they are. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Sorter as MeasurementSorter
participant SSP as StationSpacePoint
Client->>Sorter: Create sorter instance with muon point
Sorter->>SSP: Call localPosition()
Sorter->>SSP: Call sensorDirection()
Sorter->>SSP: Call stripPlaneNormal()
Sorter->>SSP: Call driftRadius()
Sorter->>SSP: Call time()
Client->>Sorter: Retrieve sorted measurements
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
Component - Core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Core/include/Acts/EventData/StationSpacePoint.hpp (1)
21-22: Complete the documentation, you must.Incomplete, this documentation is. The sentence "To describe the" hangs without purpose, like a lightsaber without its crystal.
Examples/Io/Csv/src/CsvOutputData.hpp (1)
125-128: Redundant, these covariance entries are. Symmetric matrix properties, you must respect.In a covariance matrix, covXY equals covYX it does. Unnecessary duplication of data, this creates.
Simplify the structure, you should:
float covX{0.f}; float covXY{0.f}; - float covYX{0.f}; float covY{0.f};And update the DFE_NAMEDTUPLE macro accordingly:
DFE_NAMEDTUPLE(MuonSpacePointData, sectorId, bucketId, localPositionX ,localPositionY ,localPositionZ, locSensorDirX, locSensorDirY, locSensorDirZ, locPlaneNormX, locPlaneNormY, locPlaneNormZ, - covX, covXY, covYX, covY, driftR, + covX, covXY, covY, driftR, technology, gasGap, primaryCh, secondaryCh);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Core/include/Acts/EventData/StationSpacePoint.hpp(1 hunks)Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp(1 hunks)Examples/Framework/include/ActsExamples/EventData/MuonSimHit.hpp(1 hunks)Examples/Framework/include/ActsExamples/EventData/MuonSpacePoint.hpp(1 hunks)Examples/Io/Csv/src/CsvOutputData.hpp(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Examples/Framework/include/ActsExamples/EventData/MuonSimHit.hpp
- Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: CI Bridge / lcg_107: [alma9, clang19]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_107: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: unused_files
- GitHub Check: macos
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: missing_includes
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: linux_ubuntu
- GitHub Check: build_debug
🔇 Additional comments (3)
Core/include/Acts/EventData/StationSpacePoint.hpp (1)
23-37: Well-defined concept requirements, I sense.Strong with the Force, this concept definition is. Clear requirements for space point measurements in composite detectors, it provides. Approve this implementation, I do.
Examples/Framework/include/ActsExamples/EventData/MuonSpacePoint.hpp (1)
32-36: Wise use of move semantics, I observe.The Force flows strong through these setter methods. Move semantics, they use wisely. Memory efficiency, they bring.
Also applies to: 38-40, 42-44, 46-48
Examples/Io/Csv/src/CsvOutputData.hpp (1)
106-147: Well-structured data representation, this is.Clear documentation and comprehensive fields for muon measurements, this structure provides. Strong foundation for CSV output handling, it creates.
| const Acts::Vector3& stripPlaneNormal() const { | ||
| return m_pos; | ||
| } |
There was a problem hiding this comment.
A disturbance in the Force, I sense. Wrong member variable returned, it is.
Return m_pos instead of m_norm, this function does. A critical error this is, young padawan.
Fix this, you must:
- return m_pos;
+ return m_norm;📝 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.
| const Acts::Vector3& stripPlaneNormal() const { | |
| return m_pos; | |
| } | |
| const Acts::Vector3& stripPlaneNormal() const { | |
| return m_norm; | |
| } |
junggjo9
force-pushed
the
CsvHough
branch
2 times, most recently
from
5316cea to
517d6e7
Compare
junggjo9
force-pushed
the
CsvHough
branch
2 times, most recently
from
39ab64f to
059b5e5
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
🧩 Analysis chain🏁 Script executed: Length of output: 5939 🏁 Script executed: Length of output: 46508 @junggjo9 Review your PR I will, hmm? Impressed I am with your migration of the R4-MS segment fit! Study the changes in depth, I did. Strong Points, Your Code Has
Improvement Opportunities, There Are
Questions For You, I Have
Good progress this PR shows. When ready to address these points, notify me you shall! ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
Core/include/Acts/EventData/StationSpacePoint.hpp (3)
1-45: A strong concept definition for StationSpacePoint, this is.A well-defined concept for station space points, you have created. Clear requirements for types implementing this concept, you have specified. The force of strong typing, I sense in this file.
Hmm, one small issue I observe. Incomplete concept definition at line 43-44, there is. A commented template declaration without implementation, remove or complete you should.
- /** @brief Concept of a space point container */ - // template
15-15: Missing space after namespace name, I see.Consistency in your code style, important it is. After namespace name, a space add you should.
-namespace Acts{ +namespace Acts {
23-41: Good use of concepts, but comment formatting improve you must.The concept implementation itself, strong it is. But your comments, align with Doxygen style better they should. Double asterisks at the beginning of each line in multi-line comments, use you should.
Examples/Algorithms/TrackFinding/src/MuonSegmentFinder.cpp (1)
9-13: Excessive empty lines between namespace and class, unnecessary they are.Too many empty lines, readability they do not improve. Remove some of them you should, hmm?
namespace ActsExamples{ - - MuonSegmentFinder::MuonSegmentFinder(Config cfg, Acts::Logging::Level lvl)Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonSegmentFinder.hpp (1)
21-29: Consider initializing data handles or removing the commented code.
Hmmm, see I do that lines for reading data handles are commented out. If needed, use them you should, or remove them to keep the code clean and confusion away, yes. Clarity in code, the Force demands.Core/include/Acts/Surfaces/detail/LineHelper.hpp (1)
36-46: Handle degenerate cases more explicitly, you could.
Parallel lines or near-parallel lines handle with caution, an extra threshold or fallback might be wise, yes. Return invalid intersection you do, but further logging or distinct results might help debugging. Guide others with the Force, you should.Core/include/Acts/Seeding/StrawChamberLineSeeder.ipp (1)
98-120: Implement the seed-building logic fully, helpful it would be.
Currently returnsstd::nullopt, yes? Implementation incomplete. Extend you should, or confirm that stub it remains. Improve the path of the seeding, you will.🧰 Tools
🪛 GitHub Actions: Builds
[error] 116-116: unused variable 'y0' [-Werror,-Wunused-variable]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Core/include/Acts/EventData/StationSpacePoint.hpp(1 hunks)Core/include/Acts/Seeding/StrawChamberLineSeeder.hpp(1 hunks)Core/include/Acts/Seeding/StrawChamberLineSeeder.ipp(1 hunks)Core/include/Acts/Seeding/detail/SrawChamberLineSeederUtils.hpp(1 hunks)Core/include/Acts/Surfaces/detail/LineHelper.hpp(1 hunks)Examples/Algorithms/TrackFinding/CMakeLists.txt(1 hunks)Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonHoughSeeder.hpp(3 hunks)Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonSegmentFinder.hpp(1 hunks)Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp(5 hunks)Examples/Algorithms/TrackFinding/src/MuonSegmentFinder.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Core/include/Acts/Seeding/detail/SrawChamberLineSeederUtils.hpp
🧰 Additional context used
🪛 GitHub Actions: Builds
Examples/Algorithms/TrackFinding/src/MuonSegmentFinder.cpp
[error] 29-29: unused parameter 'ctx' [-Werror,-Wunused-parameter]
Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp
[error] 57-57: unused variable 'gotSH' [-Werror,-Wunused-variable]
Core/include/Acts/Seeding/StrawChamberLineSeeder.ipp
[error] 116-116: unused variable 'y0' [-Werror,-Wunused-variable]
[error] 93-93: unused parameter 'ctx' [-Werror,-Wunused-parameter]
🪛 GitHub Actions: Docs
Core/include/Acts/Seeding/StrawChamberLineSeeder.hpp
[error] 155-155: Doxygen error: argument 'sign' of command @param is not found in the argument list of Acts::StrawChamberLineSeeder::buildSeed.
[error] 155-155: Doxygen error: The following parameters are not documented: 'ctx', 'signs'.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: missing_includes
- GitHub Check: build_debug
🔇 Additional comments (13)
Examples/Algorithms/TrackFinding/CMakeLists.txt (1)
12-12: Good, the addition of MuonSegmentFinder.cpp is, hmm.New source file to the library, properly added it has been. Consistent with the path structure and naming conventions, it is.
Examples/Algorithms/TrackFinding/src/MuonSegmentFinder.cpp (1)
30-36: Empty hits vector used, no actual functionality implemented this has.Creating a seeder with empty hits vector and default configuration, hmm? No actual work your function performs. Stub implementation this appears to be. Comment explaining future implementation, add you should.
Are you planning to implement the functionality later? Add a TODO comment if that is your intention, you should.
Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonHoughSeeder.hpp (3)
16-17: Good, the inclusion of new headers for MuonSpacePoint and MuonHoughMaximum is.Proper headers added you have. Migration from DriftCircle to MuonSpacePoint, begun it has. The path to enlightenment, this is.
52-54: Config structure updated properly, it has been.Replace DriftCircles with SpacePoints in configuration, wise decision this is. Added output for HoughMax, good foresight you have shown.
76-77: Data handles updated to match new types, well done this is.ReadDataHandle for MuonSpacePointCont and WriteDataHandle for MuonHoughMaxContainer, properly implemented they are. Consistent with config changes, this is.
Examples/Algorithms/TrackFinding/include/ActsExamples/TrackFinding/MuonSegmentFinder.hpp (1)
43-45: Ensure correct implementation of execute, initialize, and finalize.
Implemented the interface methods may be, hmm? But see no definitions here I do. Provide them externally you might. Confirm they exist, you must, so the Force remains balanced.Core/include/Acts/Surfaces/detail/LineHelper.hpp (1)
67-69: Clarify the near-zero check for plane intersection.
Caught in an epsilon check, we are. Precisely define the threshold for when a line is parallel to the plane, you must. Decrease confusion in boundary conditions, it will.Examples/Algorithms/TrackFinding/src/MuonHoughSeeder.cpp (6)
10-14: Good additions, these includes are.
Needed for muon seeding logic, I sense.
21-21: Matters not, these changes do.
Harmless new format and cutoff constant, they are.Also applies to: 28-28
32-54: Constructor changes, no conflict I see.
Structure consistent, well formed it is.
58-137: Hough transform logic, implemented well it seems.
Further validation, you might do, but concerns immediate I do not sense.
255-256: Return success, coherent is.
Fine, these lines are.
259-273: Initialization and finalization, correct they appear.
Problems here, found none I have.
| ProcessCode MuonSegmentFinder::execute( const AlgorithmContext& ctx) const { | ||
|
|
||
| using StrawSeeder_t = Acts::StrawChamberLineSeeder<MuonSpacePoint, MuonSpacePointSorter>; | ||
| StrawSeeder_t::Config seedCfg{}; | ||
| std::vector<const MuonSpacePoint*> hits{}; | ||
|
|
||
| StrawSeeder_t seeder{hits, std::move(seedCfg), m_logger->clone()}; | ||
| return ProcessCode::SUCCESS; | ||
| } |
There was a problem hiding this comment.
Unused parameter 'ctx' exists. Fix this issue, you must.
Pipeline failure warns about unused parameter 'ctx'. Properly use the parameter or mark it as unused with (void)ctx; syntax you should. Empty implementation has not much purpose, hmm?
ProcessCode MuonSegmentFinder::execute( const AlgorithmContext& ctx) const {
+ // Suppress unused parameter warning
+ (void)ctx;
using StrawSeeder_t = Acts::StrawChamberLineSeeder<MuonSpacePoint, MuonSpacePointSorter>;
StrawSeeder_t::Config seedCfg{};
std::vector<const MuonSpacePoint*> hits{};
StrawSeeder_t seeder{hits, std::move(seedCfg), m_logger->clone()};
return ProcessCode::SUCCESS;
}📝 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.
| ProcessCode MuonSegmentFinder::execute( const AlgorithmContext& ctx) const { | |
| using StrawSeeder_t = Acts::StrawChamberLineSeeder<MuonSpacePoint, MuonSpacePointSorter>; | |
| StrawSeeder_t::Config seedCfg{}; | |
| std::vector<const MuonSpacePoint*> hits{}; | |
| StrawSeeder_t seeder{hits, std::move(seedCfg), m_logger->clone()}; | |
| return ProcessCode::SUCCESS; | |
| } | |
| ProcessCode MuonSegmentFinder::execute( const AlgorithmContext& ctx) const { | |
| // Suppress unused parameter warning | |
| (void)ctx; | |
| using StrawSeeder_t = Acts::StrawChamberLineSeeder<MuonSpacePoint, MuonSpacePointSorter>; | |
| StrawSeeder_t::Config seedCfg{}; | |
| std::vector<const MuonSpacePoint*> hits{}; | |
| StrawSeeder_t seeder{hits, std::move(seedCfg), m_logger->clone()}; | |
| return ProcessCode::SUCCESS; | |
| } |
🧰 Tools
🪛 GitHub Actions: Builds
[error] 29-29: unused parameter 'ctx' [-Werror,-Wunused-parameter]
| // if (m_cfg.inDriftCircles.empty()) { | ||
| // throw std::invalid_argument( | ||
| // "MuonSegmentFinder: Missing drift circle collection"); | ||
| // } | ||
| // if (m_cfg.inSimHits.empty()) { | ||
| // throw std::invalid_argument("MuonSegmentFinder: Missing sim hit collection"); | ||
| // } | ||
|
|
||
| // m_inputDriftCircles.initialize(m_cfg.inDriftCircles); | ||
| // m_inputSimHits.initialize(m_cfg.inSimHits); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Commented code, remove or uncomment you should.
Dead code that is commented out, not good practice it is. Either remove these lines or implement proper error handling you must. Commented code, confusion it creates and maintenance it hampers.
-// if (m_cfg.inDriftCircles.empty()) {
-// throw std::invalid_argument(
-// "MuonSegmentFinder: Missing drift circle collection");
-// }
-// if (m_cfg.inSimHits.empty()) {
-// throw std::invalid_argument("MuonSegmentFinder: Missing sim hit collection");
-// }
-
-// m_inputDriftCircles.initialize(m_cfg.inDriftCircles);
-// m_inputSimHits.initialize(m_cfg.inSimHits);📝 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.
| // if (m_cfg.inDriftCircles.empty()) { | |
| // throw std::invalid_argument( | |
| // "MuonSegmentFinder: Missing drift circle collection"); | |
| // } | |
| // if (m_cfg.inSimHits.empty()) { | |
| // throw std::invalid_argument("MuonSegmentFinder: Missing sim hit collection"); | |
| // } | |
| // m_inputDriftCircles.initialize(m_cfg.inDriftCircles); | |
| // m_inputSimHits.initialize(m_cfg.inSimHits); |
| double theta{thetaTubes - std::asin(std::clamp(R / distTubes, -1., 1.))}; | ||
|
|
||
| Vector3 seedDir = makeDirectionFromPhiTheta(90.*UnitConstants::degree, theta); | ||
| double y0 = bottomPos.y() * seedDir.z() - bottomPos.z() * seedDir.y() + signBot * bottomHit->driftRadius(); |
There was a problem hiding this comment.
Remove or consume variable 'y0', you must.
Unused is the local variable, hmm. With purpose it must be, or banish it you shall to restore pipeline harmony.
- double y0 = bottomPos.y() * seedDir.z() - bottomPos.z() * seedDir.y() + signBot * bottomHit->driftRadius();
+ // Removed or used if needed for further calculations📝 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.
| double y0 = bottomPos.y() * seedDir.z() - bottomPos.z() * seedDir.y() + signBot * bottomHit->driftRadius(); | |
| // Removed or used if needed for further calculations |
🧰 Tools
🪛 GitHub Actions: Builds
[error] 116-116: unused variable 'y0' [-Werror,-Wunused-variable]
|
|
||
| template <StationSpacePoint UncalibSp_t, StationSpacePointSorter<UncalibSp_t> Sorter_t> | ||
| std::optional<typename StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::DriftCircleSeed> | ||
| StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::buildSeed(const CalibrationContext& ctx, |
There was a problem hiding this comment.
Remove or utilize parameter 'ctx' you must.
Unused the parameter is, causing pipeline fail, hmm. Either use it for calibration logic or remove it to quell compiler's wrath. Let no wasted parameter remain, yes.
- std::optional<typename StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::DriftCircleSeed>
- buildSeed(const CalibrationContext& ctx,
+ std::optional<typename StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::DriftCircleSeed>
+ buildSeed(/* const CalibrationContext& ctx, */ 📝 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.
| StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::buildSeed(const CalibrationContext& ctx, | |
| std::optional<typename StrawChamberLineSeeder<UncalibSp_t, Sorter_t>::DriftCircleSeed> | |
| buildSeed(/* const CalibrationContext& ctx, */ |
🧰 Tools
🪛 GitHub Actions: Builds
[error] 93-93: unused parameter 'ctx' [-Werror,-Wunused-parameter]
| /** @brief Tries to build the seed from the two hits. Fails if the solution is invalid | ||
| * or if the seed has already been built before | ||
| * @param topHit: Hit candidate from the upper layer | ||
| * @param bottomHit: Hit candidate from the lower layer | ||
| * @param sign: Object encoding whether the tangent is left / right */ | ||
| std::optional<DriftCircleSeed> buildSeed(const CalibrationContext& ctx, | ||
| const UncalibSp_t& topHit, | ||
| const UncalibSp_t& bottomHit, | ||
| const SignCombo_t& signs); |
There was a problem hiding this comment.
Fix Doxygen doc, you must.
Mismatch between documented param name and actual argument, I see. Also missing docs for 'ctx' and 'signs', there are.
Proposed fix, apply you can:
- /**
- * @param topHit: Hit candidate from the upper layer
- * @param bottomHit: Hit candidate from the lower layer
- * @param sign: Object encoding whether the tangent is left / right
- */
+ /**
+ * @brief Tries to build the seed from two hits. Fails if invalid solution this is,
+ * or if the seed already built, it has been.
+ * @param ctx The calibration context, used for calibrations it is.
+ * @param topHit Hit candidate from the upper layer, hmmm.
+ * @param bottomHit Hit candidate from the lower layer, correct it is.
+ * @param signs Whether tangents left/right they be, encoded here.
+ */📝 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.
| /** @brief Tries to build the seed from the two hits. Fails if the solution is invalid | |
| * or if the seed has already been built before | |
| * @param topHit: Hit candidate from the upper layer | |
| * @param bottomHit: Hit candidate from the lower layer | |
| * @param sign: Object encoding whether the tangent is left / right */ | |
| std::optional<DriftCircleSeed> buildSeed(const CalibrationContext& ctx, | |
| const UncalibSp_t& topHit, | |
| const UncalibSp_t& bottomHit, | |
| const SignCombo_t& signs); | |
| /** | |
| * @brief Tries to build the seed from two hits. Fails if invalid solution this is, | |
| * or if the seed already built, it has been. | |
| * @param ctx The calibration context, used for calibrations it is. | |
| * @param topHit Hit candidate from the upper layer, hmmm. | |
| * @param bottomHit Hit candidate from the lower layer, correct it is. | |
| * @param signs Whether tangents left/right they be, encoded here. | |
| */ | |
| std::optional<DriftCircleSeed> buildSeed(const CalibrationContext& ctx, | |
| const UncalibSp_t& topHit, | |
| const UncalibSp_t& bottomHit, | |
| const SignCombo_t& signs); |
🧰 Tools
🪛 GitHub Actions: Docs
[error] 155-155: Doxygen error: argument 'sign' of command @param is not found in the argument list of Acts::StrawChamberLineSeeder::buildSeed.
[error] 155-155: Doxygen error: The following parameters are not documented: 'ctx', 'signs'.
| const AlgorithmContext& ctx) const { | ||
| // read the hits and circles | ||
| auto gotSH = m_inputSimHits(ctx); | ||
| auto gotDC = m_inputDriftCircles(ctx); | ||
| const auto& gotSH = m_inputSimHits(ctx); |
There was a problem hiding this comment.
Unused variable, remove it you must!
Build broken is, by 'gotSH' not used. Fix quickly, you should.
Apply thus:
- const auto& gotSH = m_inputSimHits(ctx);Or cast to void, if used in future:
// (void) gotSH; 🧰 Tools
🪛 GitHub Actions: Builds
[error] 57-57: unused variable 'gotSH' [-Werror,-Wunused-variable]
PLEASE DESCRIBE YOUR CHANGES.
THIS MESSAGE ENDS UP AS THE COMMIT MESSAGE.
DO NOT USE @-MENTIONS HERE!
--- END COMMIT MESSAGE ---
@goblirsc , @dimitra97
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes