Skip to content

fix(patchworkpp): clear ringwise_flatness every ring (#69)#91

Merged
LimHyungTae merged 1 commit into
masterfrom
fix/issue-69-ringwise-flatness-clear
May 21, 2026
Merged

fix(patchworkpp): clear ringwise_flatness every ring (#69)#91
LimHyungTae merged 1 commit into
masterfrom
fix/issue-69-ringwise-flatness-clear

Conversation

@LimHyungTae

Copy link
Copy Markdown
Contributor

What

Move ringwise_flatness.clear() outside the if (!candidates.empty()) block in cpp/patchworkpp/src/patchworkpp.cpp so it runs unconditionally at the end of every ring iteration.

       if (!candidates.empty()) {
         if (params_.enable_TGR) {
           temporal_ground_revert(ringwise_flatness, candidates, concentric_idx);
         } else {
           for (auto candidate : candidates) {
             addCloud(cloud_nonground_, candidate.regionwise_ground);
           }
         }
         candidates.clear();
-        ringwise_flatness.clear();
       }
+      ringwise_flatness.clear();

Why

Reported by @KennethBlomqvist in #69. The previous placement only cleared ringwise_flatness when the current ring produced at least one revert candidate. If a ring finished with candidates empty, the accumulated flatnesses leaked into the next ring's temporal_ground_revert call and polluted its mean ± stdev statistics. Since flatness statistics differ across CZM rings (closer rings are flatter by construction), this biased the revert decision threshold (mu_flatness = mean + 1.5 * stdev, line 435).

Impact in practice is small because most rings DO produce candidates, so TGR fires every ring on KITTI. But the bug is real and the fix is one line.

Numerical impact — KITTI 00–10, full 23,201 frames, Patchwork++ paper protocol

Build P R F1
v1.3.0 baseline 95.5494 97.1649 96.2886
This PR 95.5496 97.1710 96.2918

ΔF1 = +0.003 (within run-to-run noise). Confirms no regression.

Test plan

  • One-line code change verified by reading the immediately-surrounding loop (the only callsite of temporal_ground_revert).
  • pip install -v ./python/ from a clean conda env succeeds.
  • Smoke test seq 00 first 50 frames → numbers within ±0.1 of v1.3.0.
  • Full KITTI 00–10 sweep with --method patchworkpp --eval_protocol patchworkpp → table above.

Closes #69.

ringwise_flatness was only cleared inside `if (!candidates.empty())`,
so when a ring finished with no revert candidates the accumulated
flatnesses leaked into the next ring's `temporal_ground_revert` call,
polluting its mean/stdev statistics (and therefore the revert
decision threshold) with flatnesses from a coarser/finer ring.

Move the clear outside the if-block so it runs unconditionally per
ring iteration.

Reported by @KennethBlomqvist in #69.

Closes #69.
@LimHyungTae LimHyungTae merged commit ebbc53a into master May 21, 2026
17 checks passed
@LimHyungTae LimHyungTae deleted the fix/issue-69-ringwise-flatness-clear branch May 21, 2026 03:34
LimHyungTae added a commit that referenced this pull request May 21, 2026
Patch release with one bug fix (#69 -> #91):
- ringwise_flatness is now cleared every ring (not only on rings with
  revert candidates), so stale flatnesses no longer leak into the next
  ring's TGR mean/stdev. Numerical impact on KITTI 00-10: +0.003 F1
  (within run-to-run noise).

Bumps:
- python/pyproject.toml  1.3.0 -> 1.3.1
- cpp/CMakeLists.txt     1.3.0 -> 1.3.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Question about ringwise_flatness

1 participant