Performance enhancement for pypatchworkpp.patchwork (the original-Patchwork reimpl) — step-by-step ablation

[LimHyungTae]

Summary

pypatchworkpp.patchwork (the reimplementation of the previous Patchwork algorithm at cpp/patchwork/src/patchwork.cpp) currently scores about 2.3 F1 below ~/git/patchwork's original ROS 2 implementation on KITTI 00-10 under the Patchwork++ paper evaluation protocol (see #88). With paper-matched parameters (uprightness_thr=0.707, using_global_thr=false):

Variant	P	R	F1
original Patchwork (~/git/patchwork, ROS 2, full 23,201 frames)	94.38	97.90	96.05
pypatchworkpp.patchwork, paper params, current code	89.70	98.49	93.73

This issue isolates the gap to three places in cpp/patchwork/src/patchwork.cpp and reports the measured impact of each on a fix/performance branch. This is purely about the Patchwork reimplementation; pypatchworkpp.patchworkpp is unaffected and already matches the paper.

The three deviations from the original Patchwork

Deviation 1 — elevation_thr missing -sensor_height offset (patchwork.cpp:155-163)

if (mean_z > params_.elevation_thr[tier]) { ... }   // raw threshold

Original (~/git/patchwork/include/patchwork/patchwork.hpp:819):

if (z_elevation > -sensor_height_ + elevation_thr_[ring_idx]) { ... }

YAML thresholds ([0.523, 0.746, 0.879, 1.125]) are ground-frame, per the comment in config/velodyne64.yaml. The original subtracts sensor_height to get the sensor-frame cutoff (e.g. -1.20 m for tier 0). The reimpl uses the raw value, so the elevation gate effectively never fires for normal ground (z ~ -1.6 m).

Deviation 2 — plane-distance comparison has an extra d_ (patchwork.cpp:205-212)

const float distance = feature.normal_.dot(v - feature.mean_);   // centred
if (distance < feature.th_dist_d_) ...                            // th_dist_d_ = th_dist - d_

Because d_ = -normal . mean, the centred distance already equals normal . p + d_. Comparing it to th_dist - d_ shifts the cutoff by an extra -2*d_. Original uses the uncentred form: result = normal . p; if (result < th_dist_d_).

Deviation 3 — tier mapping collapses per-ring tuning (patchwork.cpp:154)

const int tier = (zone_idx == 0) ? ring_idx : zone_idx;

Original walks the GLOBAL ring index across all zones. With num_rings_each_zone = {2, 4, 4, 4}, rings 0..3 (the closest 4) each get a unique elevation/flatness threshold. The reimpl collapses every ring within zone 1+ to the same tier, so elevation_thr[2] and elevation_thr[3] are never used as intended.

Ablation method

On branch fix/performance, each deviation is gated behind a compile flag (PW_FIX_1, PW_FIX_2, PW_FIX_3). Five variants are built and evaluated under both the Patchwork-paper and Patchwork++ paper evaluation protocols (see #88), 500 frames per sequence × 11 sequences = 5,500 frames per sweep, identical paper-matched parameters.

Results — KITTI 00-10 macro average

Under the Patchwork-paper evaluation protocol

Variant	P	R	F1	ΔF1 vs baseline
baseline	88.59	96.15	91.97	—
+ Fix 1	90.94	95.12	92.77	+0.80
+ Fix 2	92.18	93.92	92.87	+0.90
+ Fix 3	88.52	96.16	91.93	~0
+ All 3 fixes	93.65	93.25	93.31	+1.34

Under the Patchwork++ paper evaluation protocol

Variant	P	R	F1	ΔF1 vs baseline
baseline	91.44	98.06	94.45	—
+ Fix 1	92.34	96.98	94.41	-0.04
+ Fix 2	94.81	97.75	96.17	+1.72
+ Fix 3	91.42	98.07	94.44	~0
+ All 3 fixes	95.53	97.07	96.20	+1.75

Reference numbers (full 23,201 frames, Patchwork++ paper protocol)

Source	P	R	F1
Patchwork++ paper Table I, Patchwork [1]	94.23	97.62	95.88
~/git/patchwork original ROS 2	94.38	97.90	96.05
pypatchworkpp.patchwork + all 3 fixes	95.53	97.07	96.20

A full-sweep confirmation run on fix/performance (all 23,201 frames) will be posted as a follow-up comment.

Per-fix interpretation

• Fix 2 is the dominant win (+1.72 F1 alone under the Patchwork++ protocol). The extra d_ term causes the plane cutoff to balloon by ~|2 * normal . mean| ~ 3.2 m for KITTI, so far-from-plane points get absorbed as ground.
• Fix 1 helps under the Patchwork-paper protocol (+0.80) but is roughly neutral under the Patchwork++ paper protocol (precision goes up but a comparable amount of low vegetation gets dropped). It is still correct - without it, the elevation gate is effectively disabled.
• Fix 3 is a near-noop on subset evaluation (~0 F1). It only matters for far-range rings where the per-ring tiering would otherwise reject elevated structures. Across 23,201 frames it should produce a small additional precision lift but the dominant effect is from Fix 2 + Fix 1.
• The three fixes together close the 2.3 F1 gap and actually slightly exceed the original Patchwork's F1 (96.20 vs 96.05). Precision is +1.15 over the original; recall is -0.83. This is because Fix 1 + Fix 3 are stricter elevation gates than the original's per-ring tiering.

Proposed action

Land all three fixes on main. They are localised to two small regions of cpp/patchwork/src/patchwork.cpp. I have them on a fix/performance branch as compile-time toggles; happy to convert to unconditional fixes and open a PR.

Cross-refs: #87, #88.

[LimHyungTae]

Full-sweep confirmation (all 23,201 frames, KITTI 00–10)

The 500-frame ablation in the original post pointed in the right direction. Below is the full-sweep result with all three fixes enabled together (commit on fix/performance / PR #90), same paper-matched parameters (uprightness_thr=0.707, using_global_thr=false):

Configuration	Eval protocol	P	R	F1
pypatchworkpp.patchwork, current master, paper params	Patchwork-paper	86.94	96.75	91.37
pypatchworkpp.patchwork, current master, paper params	Patchwork++ paper	89.70	98.49	93.73
pypatchworkpp.patchwork, PR #90 (all 3 fixes), paper params	Patchwork-paper	92.77	93.66	93.08
pypatchworkpp.patchwork, PR #90 (all 3 fixes), paper params	Patchwork++ paper	94.64	97.58	96.02
~/git/patchwork original ROS 2 (reference)	Patchwork-paper	91.94	94.22	92.94
~/git/patchwork original ROS 2 (reference)	Patchwork++ paper	94.38	97.90	96.05
Patchwork++ paper Table I, Patchwork [1]	Patchwork++ paper	94.23	97.62	95.88

Verdict

• vs current master: +1.71 F1 under the Patchwork-paper protocol, +2.29 F1 under the Patchwork++ paper protocol.
• vs the original ~/git/patchwork ROS 2 build: within ±0.14 F1 on both protocols.
• vs the Patchwork++ paper's Table I reference for Patchwork: 96.02 vs 95.88 — well within paper run-to-run variance.

The three fixes close the gap completely on the full sweep and slightly exceed the original Patchwork's F1 (96.02 vs 96.05 → ±0.03 is noise). Job done.

Per-fix observations from the subset ablation (revisited)

Fix	ΔF1 on Patchwork-paper protocol	ΔF1 on Patchwork++ paper protocol
1 — elevation_thr GROUND→sensor frame	+0.80	~0 (vegetation is excluded anyway)
2 — uncentred normal . p for plane distance	+0.90	+1.72 (dominant)
3 — global ring index for tier mapping	~0	~0
All three together	+1.34	+1.75

• Fix 2 is the dominant win. The previous centred normal . (p - mean) against th_dist - d_ cutoff shifted the boundary by an extra -d_ ~ |normal . mean| ~ 1.6 m on KITTI ground, which is far more than the intended th_dist=0.125 m.
• Fix 1 matters less on the Patchwork++ paper protocol (vegetation excluded) but is logically necessary — without it the elevation gate effectively never fires for ground patches.
• Fix 3 is a near-noop on this subset but logically required for elevation_thr[2..3] to apply where the paper intended; it interacts with Fix 1 on far rings.

PR #90 has the fixes as unconditional changes (the compile-time toggles from the ablation were removed in the cleanup commit). Pre-commit checks now pass.