From 0984b982bc4d85d757d6bf06f911da203ca80dfc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 18:43:01 +0000 Subject: [PATCH 01/14] Initial plan From 06c09b23204fb13ecaf34bdefbbad23bd6ff9329 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 18:52:30 +0000 Subject: [PATCH 02/14] Add optional checks and enable bugprone-unchecked-optional-access Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .clang-tidy | 1 - src/algorithms/tracking/ActsToTracks.cc | 5 +++++ src/algorithms/tracking/TrackPropagation.cc | 11 ++++++++++- .../TrackingEfficiency_processor.cc | 5 +++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index b77eb7afc9..28b3f79043 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,7 +9,6 @@ Checks: ' -bugprone-easily-swappable-parameters, -bugprone-macro-parentheses, -bugprone-narrowing-conversions, - -bugprone-unchecked-optional-access, -modernize-use-trailing-return-type, -modernize-avoid-c-arrays, -modernize-use-nodiscard, diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index ce694ffc90..b30fa3288b 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -97,6 +97,11 @@ void ActsToTracks::process(const Input& input, const Output& output) const { // Get the fitted track parameter const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); + + if (!boundParam.covariance()) { + warning("No covariance matrix for trajectory with entry index = {}", trackTip); + continue; + } const auto& covariance = *boundParam.covariance(); auto pars = track_parameters->create(); diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 5582707da5..e7645a31d0 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -316,8 +316,17 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation result is OK"); // Pulling results to convenient variables - auto trackStateParams = *((*result).endParameters); + if (!(*result).endParameters) { + m_log->trace(" propagation failed (no endParameters)"); + return nullptr; + } + auto trackStateParams = *(*result).endParameters; const auto& parameter = trackStateParams.parameters(); + + if (!trackStateParams.covariance()) { + m_log->trace(" propagation failed (no covariance)"); + return nullptr; + } const auto& covariance = *trackStateParams.covariance(); // Path length diff --git a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc index ae89fbc66c..092c1c3ec6 100644 --- a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc +++ b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc @@ -108,6 +108,11 @@ void TrackingEfficiency_processor::Process(const std::shared_ptr& if (traj->hasTrackParameters(trackTip)) { const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); + + if (!boundParam.covariance()) { + m_log->debug("No covariance available for track parameters"); + continue; + } const auto& covariance = *boundParam.covariance(); m_log->debug("{:>10.2f} {:>10.2f} {:>10.2f} {:>10.3f} {:>10.4f} {:>10.3f} {:>12.4e} " "{:>12.4e} {:>12.4e} {:>8.2f}", From 09646637c76915827d8fbc047f5510b8b6ea9f57 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 18:54:41 +0000 Subject: [PATCH 03/14] Fix additional optional access issues in filtered() and predicted() calls Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- src/algorithms/tracking/TrackProjector.cc | 7 +++++++ src/algorithms/tracking/TrackPropagation.cc | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/algorithms/tracking/TrackProjector.cc b/src/algorithms/tracking/TrackProjector.cc index aaa729a410..97d7e2c106 100644 --- a/src/algorithms/tracking/TrackProjector.cc +++ b/src/algorithms/tracking/TrackProjector.cc @@ -95,7 +95,14 @@ void TrackProjector::process(const Input& input, const Output& output) const { } // get track state bound parameters and their boundCovs + if (!trackstate.predicted()) { + return; // skip this track state if no predicted parameters + } const auto& boundParams = trackstate.predicted(); + + if (!trackstate.predictedCovariance()) { + return; // skip this track state if no predicted covariance + } const auto& boundCov = trackstate.predictedCovariance(); // convert local to global diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index e7645a31d0..77b671ab2e 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -267,7 +267,17 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, // For last measurement surface, filtered and smoothed results are equivalent auto trackState = mj.getTrackState(trackTip); auto initSurface = trackState.referenceSurface().getSharedPtr(); + + if (!trackState.filtered()) { + m_log->trace(" no filtered parameters available"); + return nullptr; + } const auto& initParams = trackState.filtered(); + + if (!trackState.filteredCovariance()) { + m_log->trace(" no filtered covariance available"); + return nullptr; + } const auto& initCov = trackState.filteredCovariance(); Acts::BoundTrackParameters initBoundParams(initSurface, initParams, initCov, From 349b239bb30e03ed1f24534a13b79427a5d37c2b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 22 Aug 2025 19:02:26 +0000 Subject: [PATCH 04/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/algorithms/tracking/ActsToTracks.cc | 2 +- src/algorithms/tracking/TrackProjector.cc | 4 ++-- src/algorithms/tracking/TrackPropagation.cc | 16 ++++++++-------- .../TrackingEfficiency_processor.cc | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index b30fa3288b..ad3e022748 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -97,7 +97,7 @@ void ActsToTracks::process(const Input& input, const Output& output) const { // Get the fitted track parameter const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); - + if (!boundParam.covariance()) { warning("No covariance matrix for trajectory with entry index = {}", trackTip); continue; diff --git a/src/algorithms/tracking/TrackProjector.cc b/src/algorithms/tracking/TrackProjector.cc index 97d7e2c106..5ea622fc65 100644 --- a/src/algorithms/tracking/TrackProjector.cc +++ b/src/algorithms/tracking/TrackProjector.cc @@ -99,11 +99,11 @@ void TrackProjector::process(const Input& input, const Output& output) const { return; // skip this track state if no predicted parameters } const auto& boundParams = trackstate.predicted(); - + if (!trackstate.predictedCovariance()) { return; // skip this track state if no predicted covariance } - const auto& boundCov = trackstate.predictedCovariance(); + const auto& boundCov = trackstate.predictedCovariance(); // convert local to global auto global = trackstate.referenceSurface().localToGlobal( diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 77b671ab2e..7e29eb4ba1 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -265,20 +265,20 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, // Get track state at last measurement surface // For last measurement surface, filtered and smoothed results are equivalent - auto trackState = mj.getTrackState(trackTip); - auto initSurface = trackState.referenceSurface().getSharedPtr(); - + auto trackState = mj.getTrackState(trackTip); + auto initSurface = trackState.referenceSurface().getSharedPtr(); + if (!trackState.filtered()) { m_log->trace(" no filtered parameters available"); return nullptr; } const auto& initParams = trackState.filtered(); - + if (!trackState.filteredCovariance()) { m_log->trace(" no filtered covariance available"); return nullptr; } - const auto& initCov = trackState.filteredCovariance(); + const auto& initCov = trackState.filteredCovariance(); Acts::BoundTrackParameters initBoundParams(initSurface, initParams, initCov, Acts::ParticleHypothesis::pion()); @@ -330,9 +330,9 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation failed (no endParameters)"); return nullptr; } - auto trackStateParams = *(*result).endParameters; - const auto& parameter = trackStateParams.parameters(); - + auto trackStateParams = *(*result).endParameters; + const auto& parameter = trackStateParams.parameters(); + if (!trackStateParams.covariance()) { m_log->trace(" propagation failed (no covariance)"); return nullptr; diff --git a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc index 092c1c3ec6..bef0c4fe5f 100644 --- a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc +++ b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc @@ -108,7 +108,7 @@ void TrackingEfficiency_processor::Process(const std::shared_ptr& if (traj->hasTrackParameters(trackTip)) { const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); - + if (!boundParam.covariance()) { m_log->debug("No covariance available for track parameters"); continue; From 9a6c581ed7f49c6b4b97b858f67623c6209a760d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 19:25:15 +0000 Subject: [PATCH 05/14] Use version-conditional checks for optional access in Acts API Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- src/algorithms/tracking/TrackProjector.cc | 7 +++++-- src/algorithms/tracking/TrackPropagation.cc | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/algorithms/tracking/TrackProjector.cc b/src/algorithms/tracking/TrackProjector.cc index 5ea622fc65..4aca9a195a 100644 --- a/src/algorithms/tracking/TrackProjector.cc +++ b/src/algorithms/tracking/TrackProjector.cc @@ -95,15 +95,18 @@ void TrackProjector::process(const Input& input, const Output& output) const { } // get track state bound parameters and their boundCovs +#if Acts_VERSION_MAJOR >= 36 if (!trackstate.predicted()) { return; // skip this track state if no predicted parameters } +#endif const auto& boundParams = trackstate.predicted(); - +#if Acts_VERSION_MAJOR >= 36 if (!trackstate.predictedCovariance()) { return; // skip this track state if no predicted covariance } - const auto& boundCov = trackstate.predictedCovariance(); +#endif + const auto& boundCov = trackstate.predictedCovariance(); // convert local to global auto global = trackstate.referenceSurface().localToGlobal( diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 7e29eb4ba1..91b2b2b22b 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -265,20 +265,22 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, // Get track state at last measurement surface // For last measurement surface, filtered and smoothed results are equivalent - auto trackState = mj.getTrackState(trackTip); - auto initSurface = trackState.referenceSurface().getSharedPtr(); - + auto trackState = mj.getTrackState(trackTip); + auto initSurface = trackState.referenceSurface().getSharedPtr(); +#if Acts_VERSION_MAJOR >= 36 if (!trackState.filtered()) { m_log->trace(" no filtered parameters available"); return nullptr; } +#endif const auto& initParams = trackState.filtered(); - +#if Acts_VERSION_MAJOR >= 36 if (!trackState.filteredCovariance()) { m_log->trace(" no filtered covariance available"); return nullptr; } - const auto& initCov = trackState.filteredCovariance(); +#endif + const auto& initCov = trackState.filteredCovariance(); Acts::BoundTrackParameters initBoundParams(initSurface, initParams, initCov, Acts::ParticleHypothesis::pion()); @@ -330,9 +332,9 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation failed (no endParameters)"); return nullptr; } - auto trackStateParams = *(*result).endParameters; - const auto& parameter = trackStateParams.parameters(); - + auto trackStateParams = *(*result).endParameters; + const auto& parameter = trackStateParams.parameters(); + if (!trackStateParams.covariance()) { m_log->trace(" propagation failed (no covariance)"); return nullptr; From a94bf1466825830eeadd025876f72f08071b192e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 00:21:14 +0000 Subject: [PATCH 06/14] Revert to original state to investigate clang-tidy warnings Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .clang-tidy | 1 + src/algorithms/tracking/ActsToTracks.cc | 5 ---- src/algorithms/tracking/TrackProjector.cc | 10 -------- src/algorithms/tracking/TrackPropagation.cc | 23 +------------------ .../TrackingEfficiency_processor.cc | 5 ---- 5 files changed, 2 insertions(+), 42 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 28b3f79043..069842df90 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,6 +9,7 @@ Checks: ' -bugprone-easily-swappable-parameters, -bugprone-macro-parentheses, -bugprone-narrowing-conversions, + -modernize-use-trailing-return-type, -modernize-avoid-c-arrays, -modernize-use-nodiscard, diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index ad3e022748..ce694ffc90 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -97,11 +97,6 @@ void ActsToTracks::process(const Input& input, const Output& output) const { // Get the fitted track parameter const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); - - if (!boundParam.covariance()) { - warning("No covariance matrix for trajectory with entry index = {}", trackTip); - continue; - } const auto& covariance = *boundParam.covariance(); auto pars = track_parameters->create(); diff --git a/src/algorithms/tracking/TrackProjector.cc b/src/algorithms/tracking/TrackProjector.cc index 4aca9a195a..aaa729a410 100644 --- a/src/algorithms/tracking/TrackProjector.cc +++ b/src/algorithms/tracking/TrackProjector.cc @@ -95,17 +95,7 @@ void TrackProjector::process(const Input& input, const Output& output) const { } // get track state bound parameters and their boundCovs -#if Acts_VERSION_MAJOR >= 36 - if (!trackstate.predicted()) { - return; // skip this track state if no predicted parameters - } -#endif const auto& boundParams = trackstate.predicted(); -#if Acts_VERSION_MAJOR >= 36 - if (!trackstate.predictedCovariance()) { - return; // skip this track state if no predicted covariance - } -#endif const auto& boundCov = trackstate.predictedCovariance(); // convert local to global diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 91b2b2b22b..5582707da5 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -267,19 +267,7 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, // For last measurement surface, filtered and smoothed results are equivalent auto trackState = mj.getTrackState(trackTip); auto initSurface = trackState.referenceSurface().getSharedPtr(); -#if Acts_VERSION_MAJOR >= 36 - if (!trackState.filtered()) { - m_log->trace(" no filtered parameters available"); - return nullptr; - } -#endif const auto& initParams = trackState.filtered(); -#if Acts_VERSION_MAJOR >= 36 - if (!trackState.filteredCovariance()) { - m_log->trace(" no filtered covariance available"); - return nullptr; - } -#endif const auto& initCov = trackState.filteredCovariance(); Acts::BoundTrackParameters initBoundParams(initSurface, initParams, initCov, @@ -328,17 +316,8 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation result is OK"); // Pulling results to convenient variables - if (!(*result).endParameters) { - m_log->trace(" propagation failed (no endParameters)"); - return nullptr; - } - auto trackStateParams = *(*result).endParameters; + auto trackStateParams = *((*result).endParameters); const auto& parameter = trackStateParams.parameters(); - - if (!trackStateParams.covariance()) { - m_log->trace(" propagation failed (no covariance)"); - return nullptr; - } const auto& covariance = *trackStateParams.covariance(); // Path length diff --git a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc index bef0c4fe5f..ae89fbc66c 100644 --- a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc +++ b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc @@ -108,11 +108,6 @@ void TrackingEfficiency_processor::Process(const std::shared_ptr& if (traj->hasTrackParameters(trackTip)) { const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); - - if (!boundParam.covariance()) { - m_log->debug("No covariance available for track parameters"); - continue; - } const auto& covariance = *boundParam.covariance(); m_log->debug("{:>10.2f} {:>10.2f} {:>10.2f} {:>10.3f} {:>10.4f} {:>10.3f} {:>12.4e} " "{:>12.4e} {:>12.4e} {:>8.2f}", From 3a4def96ca7d3eeca2de6f6cce7f1fab0814a51e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 00:49:45 +0000 Subject: [PATCH 07/14] Modify IWYU step to run clang-tidy on specific PR files Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/workflows/linux-eic-shell.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index 824bafa87a..cc8b172ea2 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -251,11 +251,25 @@ jobs: with: platform-release: "${{ env.platform }}:${{ env.release }}" run: | + # Run clang-tidy on specific files to check for bugprone-unchecked-optional-access warnings + echo "Running clang-tidy on PR files to check for bugprone-unchecked-optional-access warnings..." + PR_FILES="src/algorithms/tracking/TrackPropagation.cc src/algorithms/tracking/ActsToTracks.cc src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc src/algorithms/tracking/TrackProjector.cc" + for file in $PR_FILES; do + if [[ -f "$file" ]]; then + echo "Checking $file for clang-tidy warnings..." + run-clang-tidy -p build -checks=bugprone-unchecked-optional-access -extra-arg='-std=c++20' "$file" || true + else + echo "File $file not found" + fi + done + + # Original IWYU logic on specific files + echo "Running IWYU on PR files..." # reduce headers until diff is stable while [[ ${sha:-} != $(git diff | sha256sum) ]] ; do sha=$(git diff | sha256sum) echo $sha - iwyu_tool.py -p build $(git diff --name-only ${{ github.event.pull_request.head.sha }} ${{ github.event.pull_request.base.sha }}) -- -Xiwyu --verbose=3 -Xiwyu --no_fwd_decls -Xiwyu --cxx17ns -Xiwyu --mapping_file=${{github.workspace}}/.github/iwyu.imp -Xiwyu --regex=ecmascript | tee iwyu_fixes.log + iwyu_tool.py -p build $PR_FILES -- -Xiwyu --verbose=3 -Xiwyu --no_fwd_decls -Xiwyu --cxx17ns -Xiwyu --mapping_file=${{github.workspace}}/.github/iwyu.imp -Xiwyu --regex=ecmascript | tee iwyu_fixes.log fix_includes.py --blank_lines --nosafe_headers --reorder --separate_project_includes="" --keep_iwyu_namespace_format < iwyu_fixes.log git diff | tee iwyu_fixes.patch done From c2623ad7a095c5c05f4464494bab5a8b0865a5bb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 23 Aug 2025 00:51:34 +0000 Subject: [PATCH 08/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .github/workflows/linux-eic-shell.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index cc8b172ea2..12eccbf64b 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -262,7 +262,7 @@ jobs: echo "File $file not found" fi done - + # Original IWYU logic on specific files echo "Running IWYU on PR files..." # reduce headers until diff is stable From 2da8d9fe9f61c1201563157298186cd84920f9a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 01:29:27 +0000 Subject: [PATCH 09/14] Move clang-tidy check from IWYU step to clang-tidy step Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/workflows/linux-eic-shell.yml | 30 +++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index 12eccbf64b..c76889d05a 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -223,6 +223,19 @@ jobs: with: platform-release: "${{ env.platform }}:${{ env.release }}" run: | + # Run clang-tidy on specific files to check for bugprone-unchecked-optional-access warnings + echo "Running clang-tidy on PR files to check for bugprone-unchecked-optional-access warnings..." + PR_FILES="src/algorithms/tracking/TrackPropagation.cc src/algorithms/tracking/ActsToTracks.cc src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc src/algorithms/tracking/TrackProjector.cc" + for file in $PR_FILES; do + if [[ -f "$file" ]]; then + echo "Checking $file for clang-tidy warnings..." + run-clang-tidy -p build -checks=bugprone-unchecked-optional-access -extra-arg='-std=c++20' "$file" || true + else + echo "File $file not found" + fi + done + + # Original clang-tidy logic on changed files git diff ${{ github.event.pull_request.head.sha }} ${{ github.event.pull_request.base.sha }} | clang-tidy-diff -p 1 -path build -quiet -export-fixes clang_tidy_fixes.yaml -extra-arg='-std=c++20' -clang-tidy-binary run-clang-tidy - name: Run clang-tidy on all files uses: eic/run-cvmfs-osg-eic-shell@main @@ -251,25 +264,12 @@ jobs: with: platform-release: "${{ env.platform }}:${{ env.release }}" run: | - # Run clang-tidy on specific files to check for bugprone-unchecked-optional-access warnings - echo "Running clang-tidy on PR files to check for bugprone-unchecked-optional-access warnings..." - PR_FILES="src/algorithms/tracking/TrackPropagation.cc src/algorithms/tracking/ActsToTracks.cc src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc src/algorithms/tracking/TrackProjector.cc" - for file in $PR_FILES; do - if [[ -f "$file" ]]; then - echo "Checking $file for clang-tidy warnings..." - run-clang-tidy -p build -checks=bugprone-unchecked-optional-access -extra-arg='-std=c++20' "$file" || true - else - echo "File $file not found" - fi - done - - # Original IWYU logic on specific files - echo "Running IWYU on PR files..." + git diff ${{ github.event.pull_request.head.sha }} ${{ github.event.pull_request.base.sha }} --name-only | tee changed_files.txt # reduce headers until diff is stable while [[ ${sha:-} != $(git diff | sha256sum) ]] ; do sha=$(git diff | sha256sum) echo $sha - iwyu_tool.py -p build $PR_FILES -- -Xiwyu --verbose=3 -Xiwyu --no_fwd_decls -Xiwyu --cxx17ns -Xiwyu --mapping_file=${{github.workspace}}/.github/iwyu.imp -Xiwyu --regex=ecmascript | tee iwyu_fixes.log + iwyu_tool.py -p build $(cat changed_files.txt | grep -E '\.(cpp|cc|cxx|c\+\+)$' | paste -sd ' ') -- -Xiwyu --verbose=3 -Xiwyu --no_fwd_decls -Xiwyu --cxx17ns -Xiwyu --mapping_file=${{github.workspace}}/.github/iwyu.imp -Xiwyu --regex=ecmascript | tee iwyu_fixes.log fix_includes.py --blank_lines --nosafe_headers --reorder --separate_project_includes="" --keep_iwyu_namespace_format < iwyu_fixes.log git diff | tee iwyu_fixes.patch done From eac73f449bcbba93aef1572a7ad08935d8486b08 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 02:25:24 +0000 Subject: [PATCH 10/14] Fix quoting issue in clang-tidy workflow step Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/workflows/linux-eic-shell.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index c76889d05a..b6328e8bfa 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -225,8 +225,13 @@ jobs: run: | # Run clang-tidy on specific files to check for bugprone-unchecked-optional-access warnings echo "Running clang-tidy on PR files to check for bugprone-unchecked-optional-access warnings..." - PR_FILES="src/algorithms/tracking/TrackPropagation.cc src/algorithms/tracking/ActsToTracks.cc src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc src/algorithms/tracking/TrackProjector.cc" - for file in $PR_FILES; do + PR_FILES=( + "src/algorithms/tracking/TrackPropagation.cc" + "src/algorithms/tracking/ActsToTracks.cc" + "src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc" + "src/algorithms/tracking/TrackProjector.cc" + ) + for file in "${PR_FILES[@]}"; do if [[ -f "$file" ]]; then echo "Checking $file for clang-tidy warnings..." run-clang-tidy -p build -checks=bugprone-unchecked-optional-access -extra-arg='-std=c++20' "$file" || true From eb4deb2ac48d7e208f3f408eafae0220ee784ac2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 03:57:54 +0000 Subject: [PATCH 11/14] Fix unchecked optional access warnings in tracking algorithms Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- src/algorithms/tracking/ActsToTracks.cc | 4 ++++ src/algorithms/tracking/TrackPropagation.cc | 8 ++++++++ .../tracking_efficiency/TrackingEfficiency_processor.cc | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index ce694ffc90..561f352cf0 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -97,6 +97,10 @@ void ActsToTracks::process(const Input& input, const Output& output) const { // Get the fitted track parameter const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); + if (!boundParam.covariance().has_value()) { + m_log->warn("Track parameters do not have covariance matrix, skipping track"); + continue; + } const auto& covariance = *boundParam.covariance(); auto pars = track_parameters->create(); diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 5582707da5..0699ff5fda 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -316,8 +316,16 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation result is OK"); // Pulling results to convenient variables + if (!(*result).endParameters.has_value()) { + m_log->trace(" propagation failed (endParameters not available)"); + return nullptr; + } auto trackStateParams = *((*result).endParameters); const auto& parameter = trackStateParams.parameters(); + if (!trackStateParams.covariance().has_value()) { + m_log->trace(" propagation failed (covariance not available)"); + return nullptr; + } const auto& covariance = *trackStateParams.covariance(); // Path length diff --git a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc index ae89fbc66c..b6374cddd8 100644 --- a/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc +++ b/src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc @@ -108,6 +108,10 @@ void TrackingEfficiency_processor::Process(const std::shared_ptr& if (traj->hasTrackParameters(trackTip)) { const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); + if (!boundParam.covariance().has_value()) { + m_log->debug("Track parameters do not have covariance matrix"); + continue; + } const auto& covariance = *boundParam.covariance(); m_log->debug("{:>10.2f} {:>10.2f} {:>10.2f} {:>10.3f} {:>10.4f} {:>10.3f} {:>12.4e} " "{:>12.4e} {:>12.4e} {:>8.2f}", From 4bd06c36c374ca38438bfe54fe86c76098a7fb99 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 23 Aug 2025 04:01:59 +0000 Subject: [PATCH 12/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/algorithms/tracking/TrackPropagation.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/algorithms/tracking/TrackPropagation.cc b/src/algorithms/tracking/TrackPropagation.cc index 0699ff5fda..ed393ea76c 100644 --- a/src/algorithms/tracking/TrackPropagation.cc +++ b/src/algorithms/tracking/TrackPropagation.cc @@ -320,8 +320,8 @@ TrackPropagation::propagate(const edm4eic::Track& /* track */, m_log->trace(" propagation failed (endParameters not available)"); return nullptr; } - auto trackStateParams = *((*result).endParameters); - const auto& parameter = trackStateParams.parameters(); + auto trackStateParams = *((*result).endParameters); + const auto& parameter = trackStateParams.parameters(); if (!trackStateParams.covariance().has_value()) { m_log->trace(" propagation failed (covariance not available)"); return nullptr; From 28b7f2ce3036ea4e5638cca3ec559b60056f4539 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Fri, 22 Aug 2025 23:11:53 -0500 Subject: [PATCH 13/14] fix warning logging in ActsToTracks --- src/algorithms/tracking/ActsToTracks.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/algorithms/tracking/ActsToTracks.cc b/src/algorithms/tracking/ActsToTracks.cc index 561f352cf0..64485f3a35 100644 --- a/src/algorithms/tracking/ActsToTracks.cc +++ b/src/algorithms/tracking/ActsToTracks.cc @@ -98,7 +98,7 @@ void ActsToTracks::process(const Input& input, const Output& output) const { const auto& boundParam = traj->trackParameters(trackTip); const auto& parameter = boundParam.parameters(); if (!boundParam.covariance().has_value()) { - m_log->warn("Track parameters do not have covariance matrix, skipping track"); + warning("Track parameters do not have covariance matrix, skipping track"); continue; } const auto& covariance = *boundParam.covariance(); From 7d054473026f35d18d5589bec708598092c4bbe8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 23 Aug 2025 04:22:22 +0000 Subject: [PATCH 14/14] Revert clang-tidy workflow to original state Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com> --- .github/workflows/linux-eic-shell.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index b6328e8bfa..7e6f9af9ba 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -223,24 +223,6 @@ jobs: with: platform-release: "${{ env.platform }}:${{ env.release }}" run: | - # Run clang-tidy on specific files to check for bugprone-unchecked-optional-access warnings - echo "Running clang-tidy on PR files to check for bugprone-unchecked-optional-access warnings..." - PR_FILES=( - "src/algorithms/tracking/TrackPropagation.cc" - "src/algorithms/tracking/ActsToTracks.cc" - "src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc" - "src/algorithms/tracking/TrackProjector.cc" - ) - for file in "${PR_FILES[@]}"; do - if [[ -f "$file" ]]; then - echo "Checking $file for clang-tidy warnings..." - run-clang-tidy -p build -checks=bugprone-unchecked-optional-access -extra-arg='-std=c++20' "$file" || true - else - echo "File $file not found" - fi - done - - # Original clang-tidy logic on changed files git diff ${{ github.event.pull_request.head.sha }} ${{ github.event.pull_request.base.sha }} | clang-tidy-diff -p 1 -path build -quiet -export-fixes clang_tidy_fixes.yaml -extra-arg='-std=c++20' -clang-tidy-binary run-clang-tidy - name: Run clang-tidy on all files uses: eic/run-cvmfs-osg-eic-shell@main