fix(patchwork): close the 2.3 F1 gap vs the original Patchwork (+ USAGE.md) (#90)

* fix(patchwork): three deviations vs original Patchwork (gated by compile flags)

cpp/patchwork/src/patchwork.cpp deviates from the original
~/git/patchwork in three places that together cost about 2.3 F1 on
KITTI 00-10 under the Patchwork++ paper evaluation protocol with
paper-matched parameters (uprightness_thr=0.707, using_global_thr=false).

The fixes are gated behind PW_FIX_1, PW_FIX_2, PW_FIX_3 compile flags so
an ablation can measure each independently:

  PW_FIX_1 — elevation_thr is GROUND-frame; subtract sensor_height to
    get the sensor-frame cutoff (the YAML in ~/git/patchwork documents
    this explicitly; the reimpl was using the raw value).

  PW_FIX_2 — plane-distance comparison: original uses uncentred
    `normal . p` against `th_dist - d_`. The reimpl uses the centred
    `normal . (p - mean)` against the same threshold, which shifts the
    cutoff by an extra `-2 * d_` (about 3.2 m on KITTI ground).

  PW_FIX_3 — tier index for elevation/flatness uses the GLOBAL ring
    index across all zones, not the local ring-or-zone-id collapse.

Full sweep on KITTI 00-10 (23,201 frames), Patchwork++ paper protocol,
paper params:

  current main      : P=89.70 R=98.49 F1=93.73
  fix/performance   : P=94.64 R=97.58 F1=96.02
  ~/git/patchwork   : P=94.38 R=97.90 F1=96.05  (reference)
  paper Table I     : P=94.23 R=97.62 F1=95.88

Build a variant with:

  pip install --no-build-isolation --force-reinstall \\
    --config-settings=cmake.define.PW_FIX_1=ON \\
    --config-settings=cmake.define.PW_FIX_2=ON \\
    --config-settings=cmake.define.PW_FIX_3=ON \\
    ./python/

See #89 for the full ablation report.

Also adds:
- python/examples/evaluate_semantickitti.py — KITTI eval driver with
  both Patchwork-paper and Patchwork++ paper protocols (#87, #88).
- python/examples/aggregate_original_patchwork.py — aggregates the
  per-sequence txt files from ~/git/patchwork's eval mode.
- scripts/ablation_sweep.sh — the 5-variant ablation runner.

* fix(patchwork): apply the three Patchwork deviations unconditionally + USAGE.md

The previous commit on this branch gated the three fixes behind compile
flags (PW_FIX_1/2/3) for ablation. Measurement showed all three are
needed to match the original Patchwork; this commit removes the toggles
and applies the fixes unconditionally:

  1. tier index = global ring index across all zones (not the previous
     `(zone==0) ? ring : zone` collapse).
  2. elevation_thr is converted to the sensor frame by subtracting
     sensor_height (matches the comment in the original
     config/velodyne64.yaml).
  3. plane-distance comparison uses uncentred `normal . p` so the cutoff
     of `th_dist - d_` actually represents "signed distance < th_dist"
     (the previous centred form shifted the cutoff by an extra -d_).

The ablation runner (scripts/ablation_sweep.sh) is removed since it
referenced the compile flags.

Adds USAGE.md covering:
  - the two SemanticKITTI evaluation protocols (Patchwork vs Patchwork++
    paper) and which to pick when reproducing each paper's Table I,
  - parameter tuning order (sensor_height -> uprightness_thr -> range
    bounds -> plane-fit thresholds -> elevation_thr / RNR / RVPF / TGR),
  - a copy-pasteable command to reproduce Patchwork++ Table I.

See #87, #88, #89.

* style: apply pre-commit auto-fixes (black, isort, mdformat) and drop unused import

* chore(release): v1.3.0 with performance enhancement for pypatchworkpp.patchwork

Bumps:
- python/pyproject.toml  1.2.0 -> 1.3.0
- cpp/CMakeLists.txt     1.2.0 -> 1.3.0

Adds CHANGELOG.md with a v1.3.0 entry documenting the three performance
fixes in cpp/patchwork/src/patchwork.cpp (full sweep: F1 93.73 -> 96.02
under the Patchwork++ paper evaluation protocol; matches the original
Patchwork ROS 2 build and paper Table I within paper variance).

pypatchworkpp.patchworkpp is unaffected.

See #87, #88, #89, #90.

Author: LimHyungTae

CHANGELOG.md

@@ -0,0 +1,60 @@
+# Changelog
+
+## v1.3.0
+
+### Performance enhancement — `pypatchworkpp.patchwork` (classic Patchwork reimpl)
+
+Three deviations between `cpp/patchwork/src/patchwork.cpp` and the original
+Patchwork (`url-kaist/patchwork`) were identified and fixed. With paper-matched
+parameters (`uprightness_thr=0.707`, `using_global_thr=false`) on SemanticKITTI
+sequences 00–10 (23,201 frames), under the Patchwork++ paper evaluation
+protocol (Sec. IV.A — VEGETATION excluded):
+
+| Configuration | Precision | Recall | F1 |
+| --- | --- | --- | --- |
+| v1.2.0 (`pypatchworkpp.patchwork`) | 89.70 | 98.49 | 93.73 |
+| **v1.3.0 (`pypatchworkpp.patchwork`)** | **94.64** | **97.58** | **96.02** |
+| Original Patchwork ROS 2 (reference) | 94.38 | 97.90 | 96.05 |
+| Patchwork++ paper Table I, Patchwork \[1\] | 94.23 | 97.62 | 95.88 |
+
+**+2.29 F1** vs v1.2.0; within ±0.14 F1 of the original Patchwork ROS 2 build
+and within paper run-to-run variance of Table I.
+
+Fixes:
+
+1. `elevation_thr` is now converted to the sensor frame by subtracting
+   `sensor_height` (the YAML in the original repo documents these as
+   ground-frame). Previously the raw value was used, so the elevation gate
+   effectively never fired for normal ground.
+1. Plane-distance comparison now uses uncentred `normal · p` against
+   `th_dist_d_ = th_dist − d_`, which is equivalent to "signed distance to
+   plane \< th_dist". The previous centred form shifted the cutoff by an
+   extra `−d_ ≈ |normal · mean| ≈ 1.6 m` on KITTI ground.
+1. The elevation/flatness tier index is now the GLOBAL ring index across all
+   zones, so each of the first `elevation_thr.size()` rings gets its own
+   threshold. The previous `(zone==0) ? ring : zone` collapse destroyed the
+   per-ring tuning for zones 1+.
+
+`pypatchworkpp.patchworkpp` (the actual Patchwork++) is **unaffected** —
+all three deviations were in `cpp/patchwork/`, not `cpp/patchworkpp/`.
+
+### Documentation
+
+- Added `USAGE.md` covering (1) the two SemanticKITTI evaluation protocols
+  and which to pick when reproducing each paper's Table I, (2) the parameter
+  tuning order for a new sensor, (3) a copy-pasteable command to reproduce
+  Patchwork++ Table I.
+- Added `python/examples/evaluate_semantickitti.py` (a paper-faithful
+  evaluation driver with `--eval_protocol {patchwork, patchworkpp}`) and
+  `python/examples/aggregate_original_patchwork.py`.
+
+### References
+
+- #87 — How to reproduce the performance on the paper?
+- #88 — Explanation about the evaluation protocol
+- #89 — Performance enhancement step-by-step ablation
+- #90 — PR landing the three fixes
+
+## v1.2.0 and earlier
+
+See the git log.

USAGE.md

@@ -0,0 +1,154 @@
+# Patchwork++ — Usage Guide
+
+This guide covers three things that are easy to get wrong on first contact:
+
+1. [Choosing a SemanticKITTI evaluation protocol](#1-evaluation-protocols) — picks the right ground-truth definition so numbers match the paper.
+1. [Tuning the algorithm parameters for your sensor](#2-parameter-tuning) — what each knob does and which ones to touch first when results look bad.
+1. [Reproducing the paper's Table I](#3-reproducing-paper-table-i) — a one-command sweep.
+
+For a quick start, jump to [§3](#3-reproducing-paper-table-i).
+
+______________________________________________________________________
+
+## 1. Evaluation protocols
+
+The Patchwork and Patchwork++ papers use **different** ground-truth definitions on SemanticKITTI. The eval driver `python/examples/evaluate_semantickitti.py` supports both via `--eval_protocol {patchwork, patchworkpp}`.
+
+### A. `--eval_protocol patchwork`  (original Patchwork repo protocol)
+
+- **Ground GT** = `{ROAD (40), PARKING (44), SIDEWALK (48), OTHER_GROUND (49), LANE_MARKING (60), VEGETATION (70, only if z < −1.30 m), TERRAIN (72)}`
+- VEGETATION above −1.30 m → counts as **non-ground**.
+- UNLABELED (0) and OUTLIER (1) can be excluded from the precision denominator (`--consider_outliers`, default on).
+- Source: Patchwork paper, *"the points annotated with selected classes, i.e. lane marking, road, parking, sidewalk, other ground, vegetation, and terrain, are considered to be ground-truth ground points... only points whose z values are below −1.3 m with respect to the sensor frame are considered as ground truths"*.
+
+Use this when comparing against numbers from the **original Patchwork paper / `url-kaist/patchwork`**.
+
+### B. `--eval_protocol patchworkpp`  (Patchwork++ paper Table I protocol — DEFAULT for reproducing the Patchwork++ paper)
+
+- **Ground GT** = `{ROAD, PARKING, SIDEWALK, OTHER_GROUND, LANE_MARKING, TERRAIN}` — **no VEGETATION**.
+- VEGETATION, UNLABELED, OUTLIER are **fully excluded** from both numerator and denominator.
+- Source: Patchwork++ paper Sec. IV.A, *"unlike our previous work, the points labeled as vegetation are not evaluated as ground nor non-ground points exceptionally because it is impractical to regard the vegetation as a single ground or non-ground class. Note that this implies the points labeled as vegetation are only excluded in the evaluation step; the points are still included in the input point cloud"*.
+
+Use this when comparing against numbers from the **Patchwork++ paper**.
+
+### Why it matters
+
+Same Patchwork++ inference, KITTI 00–10 macro average, two protocols:
+
+| Protocol | Precision | Recall | F1 |
+|---|---|---|---|
+| `--eval_protocol patchwork` | 93.72 | 92.33 | 92.87 |
+| `--eval_protocol patchworkpp` | **95.55** | **97.16** | **96.29** |
+| Patchwork++ paper Table I | 94.92 | 98.18 | 96.51 |
+
+3.4 F1 difference, entirely from the protocol switch. If your reproduction is 3 F1 low, this is almost certainly the cause.
+
+______________________________________________________________________
+
+## 2. Parameter tuning
+
+If results look wrong on a new sensor (Velodyne 16/32, Ouster 64/128, Livox, etc.), tune in roughly this order. Defaults are in `cpp/patchworkpp/include/patchwork/patchworkpp.h` (Patchwork++) and `cpp/patchwork/include/patchwork/patchwork.h` (classic Patchwork).
+
+### Step 1 — Get `sensor_height` right (the most important parameter)
+
+`sensor_height` is the **height of the LiDAR origin above the ground** when the vehicle is stationary on flat pavement.
+
+- KITTI / HDL-64E on a passenger car: `1.723` m (default).
+- Ouster OS0-128 on a UGV: typically 0.6–1.0 m.
+- Livox Mid-360 on a quadruped: typically 0.3–0.6 m.
+
+**How to tell it is wrong**: precision is fine on far-range patches but ground points near the sensor are split between ground and non-ground in a striped pattern. The elevation threshold and adaptive seed selection both reference `sensor_height` directly.
+
+If you cannot measure it, leave `ATAT_ON = true` and the All-Terrain Automatic heighT estimator will recover it from the first scan.
+
+### Step 2 — Tune `uprightness_thr` for the surface roughness you expect
+
+`uprightness_thr` is the cosine of the maximum tilt angle accepted for a patch's normal vs. world-up. Higher = stricter.
+
+| Setting | Max tilt | When to use |
+|---|---|---|
+| 0.5 | ~60° | very rough terrain, off-road; library default for Patchwork++ |
+| **0.707** | **~45°** | **Patchwork paper / on-road / structured driving — recommended for KITTI** |
+| 0.866 | ~30° | flat indoor floors, parking lots |
+
+If precision is low and you see ramps, low walls, or curbs being labelled as ground: increase to 0.707 or 0.866.
+If recall is low on hills, ramps, or rough pavement: lower to 0.5 or 0.4.
+
+### Step 3 — Set range bounds `min_range` / `max_range`
+
+- `min_range` (default 2.7 m): exclude the cone right under the sensor where points are noisy (vehicle body, multipath). Decrease only if your sensor mount is very low.
+- `max_range` (default 80.0 m): the cap of the concentric zone model. The CZM zone sizes scale with this; resetting it requires rebuilding `min_ranges`. For most rotating LiDARs leave at 80 m.
+
+### Step 4 — Tune the plane-fit thresholds
+
+- `th_seeds` (default 0.5 m): a point is an LPR seed if its `z` is within `th_seeds` of the lowest-z mean. Larger → more seeds → tolerates undulating ground but admits more outliers. Lower for very flat scenes.
+- `th_dist` (default 0.125 m): distance from the fitted plane below which a point counts as ground. **This is the single biggest precision/recall knob.** Increase (0.15–0.2 m) on rough/sloped ground if recall is low. Decrease (0.05–0.1 m) on parking lots if precision is low.
+- `num_iter` (default 3): plane-refit iterations per patch. 2–3 is enough; more is wasted CPU.
+
+### Step 5 — `elevation_thr` and `flatness_thr` (only if you've changed the sensor mount or scene scale)
+
+`elevation_thr = {0.523, 0.746, 0.879, 1.125}` are the **ground-frame** height cutoffs for the four closest CZM rings — patches whose mean is more than this above the ground are rejected unless their planarity (`flatness_thr`) saves them. The library converts these to sensor-frame internally by subtracting `sensor_height`.
+
+Rule of thumb: scale them ∝ `expected_terrain_undulation / 1.723 m` if your sensor sits lower or higher than KITTI. Most users do **not** need to touch these.
+
+### Step 6 — Patchwork++ extras (`pypatchworkpp.patchworkpp` only)
+
+- `enable_RNR` (default true) — Reflected Noise Removal. Turn off only if your sensor has very clean returns near the bottom rings (most rotating LiDARs need it on).
+- `enable_RVPF` (default true) — Region-wise Vertical Plane Fitting. Helps on retaining walls / curbs. Keep on.
+- `enable_TGR` (default true) — Temporal Ground Revert. Reverts FN under-segmentation. Keep on.
+- `RNR_intensity_thr` (default 0.2) — RNR's intensity gate. Calibrate to your sensor's intensity scale: if intensities are 0–255, set to ~50.
+
+______________________________________________________________________
+
+## 3. Reproducing paper Table I
+
+```bash
+# 1. Install once
+pip install -v ./python/
+
+# 2. Reproduce Patchwork++ Table I row on KITTI 00–10
+python python/examples/evaluate_semantickitti.py \
+    --method patchworkpp \
+    --eval_protocol patchworkpp \
+    --dataset_path /path/to/SemanticKITTI/sequences \
+    --output_csv summary_patchworkpp.csv
+```
+
+Expected output (full sweep, 23,201 frames):
+
+| seq | frames | P | R | F1 |
+|---|---|---|---|---|
+| Avg | 23201 | **95.55** | **97.16** | **96.29** |
+
+Paper Table I: P=94.92, R=98.18, F1=96.51 — match within ±0.22 F1.
+
+### Quick smoke test (3 frames per seq, ~5 s total)
+
+```bash
+python python/examples/evaluate_semantickitti.py \
+    --method patchworkpp \
+    --eval_protocol patchworkpp \
+    --dataset_path /path/to/SemanticKITTI/sequences \
+    --max_frames 3 --verbose
+```
+
+### Apples-to-apples vs. the original Patchwork repo
+
+```bash
+# Compare the in-repo classic Patchwork against the original ROS 2 patchwork
+python python/examples/evaluate_semantickitti.py \
+    --method patchwork \
+    --eval_protocol patchworkpp \
+    --dataset_path /path/to/SemanticKITTI/sequences \
+    --output_csv summary_patchwork.csv
+```
+
+`--method patchwork` will be paper-faithful after the fixes on this branch (#89) land — until then it is ~2.3 F1 below the original Patchwork on the same protocol.
+
+______________________________________________________________________
+
+## See also
+
+- [`python/examples/demo_visualize.py`](python/examples/demo_visualize.py) — single-frame visualisation.
+- [`python/examples/demo_sequential.py`](python/examples/demo_sequential.py) — iterate over a folder of `.bin` files.
+- Issues: [#87](https://github.com/url-kaist/patchwork-plusplus/issues/87) (reproduce paper), [#88](https://github.com/url-kaist/patchwork-plusplus/issues/88) (evaluation protocol), [#89](https://github.com/url-kaist/patchwork-plusplus/issues/89) (performance enhancement).

cpp/CMakeLists.txt

@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.11)
-project(patchworkpp VERSION 1.2.0)
+project(patchworkpp VERSION 1.3.0)
 
 option(USE_SYSTEM_EIGEN3 "Use system pre-installed Eigen" OFF)
 option(INCLUDE_CPP_EXAMPLES "Include C++ example codes, which require Open3D for visualization" OFF)

cpp/patchwork/src/patchwork.cpp

@@ -150,11 +150,19 @@ PatchStatus PatchWork::determine_gle_status(int zone_idx,
     return PatchStatus::TooTilted;
   }
 
-  // The first elevation_thr.size() tiers get tier-specific elevation/flatness thresholds.
-  const int tier = (zone_idx == 0) ? ring_idx : zone_idx;
+  // Use the GLOBAL ring index across all zones for tier lookup so that
+  // each of the first elevation_thr.size() rings gets its own threshold,
+  // matching the original Patchwork.
+  int tier = ring_idx;
+  for (int z = 0; z < zone_idx; ++z) tier += params_.num_rings_each_zone[z];
+
   if (tier < static_cast<int>(params_.elevation_thr.size())) {
     const double mean_z = feature.mean_(2);
-    if (mean_z > params_.elevation_thr[tier]) {
+    // elevation_thr is GROUND-frame (see config/velodyne64.yaml in the
+    // original Patchwork repo); convert to the sensor frame by
+    // subtracting sensor_height.
+    const double elev_cut = -params_.sensor_height + params_.elevation_thr[tier];
+    if (mean_z > elev_cut) {
       // Recoverable if the patch is very flat
       if (feature.singular_values_(2) < params_.flatness_thr[tier]) {
         return PatchStatus::FlatEnough;
@@ -204,8 +212,13 @@ void PatchWork::perform_regionwise_segmentation(int zone_idx,
     std::vector<PointXYZ> nonground;
     for (const auto& p : sorted) {
       Eigen::Vector3f v(p.x, p.y, p.z);
-      const float distance = feature.normal_.dot(v - feature.mean_);
-      if (distance < feature.th_dist_d_) {
+      // Original Patchwork compares the uncentred  normal . p  to
+      // th_dist_d_ = th_dist - d_, which is equivalent to "signed
+      // distance to plane < th_dist".  The previous centred form here
+      // shifted the cutoff by an extra -d_ ~ |normal . mean|, which on
+      // KITTI ground is ~1.6 m and effectively disabled the cutoff.
+      const float signed_dist = feature.normal_.dot(v);
+      if (signed_dist < feature.th_dist_d_) {
         ground.push_back(p);
       } else {
         nonground.push_back(p);

python/examples/aggregate_original_patchwork.py

@@ -0,0 +1,126 @@
+"""Aggregate per-sequence txt outputs produced by the original Patchwork
+(`~/git/patchwork`) eval mode into a summary CSV matching the format produced
+by `evaluate_semantickitti.py`.
+
+Per-frame row format in ~/patchwork/<seq>.txt:
+    Legacy (6 cols): frame_idx, time, precision, recall, precision_naive, recall_naive
+    New (8 cols):    + precision_pp, recall_pp   (Patchwork++ paper protocol, Sec IV.A)
+"""
+
+import argparse
+import csv
+import os
+
+import numpy as np
+
+
+def f1(p: float, r: float) -> float:
+    return 2.0 * p * r / (p + r) if (p + r) > 0 else 0.0
+
+
+def aggregate_seq(path: str, protocol: str = "patchwork") -> dict:
+    rows = np.loadtxt(path, delimiter=",")
+    if rows.ndim == 1:
+        rows = rows.reshape(1, -1)
+    if protocol == "patchworkpp":
+        if rows.shape[1] < 8:
+            raise ValueError(
+                f"{path} only has {rows.shape[1]} columns; rerun patchwork "
+                "with the patched main.cpp/utils.hpp to produce the 8-col format."
+            )
+        p = rows[:, 6]
+        r = rows[:, 7]
+        p_n = rows[:, 6]
+        r_n = rows[:, 7]
+    else:
+        p = rows[:, 2]
+        r = rows[:, 3]
+        p_n = rows[:, 4]
+        r_n = rows[:, 5]
+    return {
+        "num_frames": int(rows.shape[0]),
+        "precision": float(p.mean()),
+        "recall": float(r.mean()),
+        "f1": float(np.mean([f1(pi, ri) for pi, ri in zip(p, r)])),
+        "precision_naive": float(p_n.mean()),
+        "recall_naive": float(r_n.mean()),
+        "f1_naive": float(np.mean([f1(pi, ri) for pi, ri in zip(p_n, r_n)])),
+    }
+
+
+def main():
+    ap = argparse.ArgumentParser(description=__doc__)
+    ap.add_argument("--txt_dir", default=os.path.expanduser("~/patchwork"))
+    ap.add_argument(
+        "--output_csv",
+        default="/home/url/git/patchwork-plusplus/summary_patchwork_original.csv",
+    )
+    ap.add_argument("--seqs", nargs="+", default=[f"{i:02d}" for i in range(11)])
+    ap.add_argument(
+        "--protocol", choices=["patchwork", "patchworkpp"], default="patchwork"
+    )
+    args = ap.parse_args()
+
+    rows: list[tuple[str, dict]] = []
+    for seq in args.seqs:
+        path = os.path.join(args.txt_dir, f"{seq}.txt")
+        if not os.path.isfile(path):
+            print(f"[WARN] missing {path}")
+            continue
+        rows.append((seq, aggregate_seq(path, args.protocol)))
+
+    avg = {
+        k: float(np.mean([m[k] for _, m in rows]))
+        for k in (
+            "precision",
+            "recall",
+            "f1",
+            "precision_naive",
+            "recall_naive",
+            "f1_naive",
+        )
+    }
+    avg["num_frames"] = int(sum(m["num_frames"] for _, m in rows))
+    rows.append(("Avg", avg))
+
+    with open(args.output_csv, "w", newline="") as fp:
+        w = csv.writer(fp)
+        w.writerow(
+            [
+                "seq",
+                "num_frames",
+                "precision",
+                "recall",
+                "f1",
+                "precision_naive",
+                "recall_naive",
+                "f1_naive",
+            ]
+        )
+        for name, m in rows:
+            w.writerow(
+                [
+                    name,
+                    m["num_frames"],
+                    f"{m['precision']:.4f}",
+                    f"{m['recall']:.4f}",
+                    f"{m['f1']:.4f}",
+                    f"{m['precision_naive']:.4f}",
+                    f"{m['recall_naive']:.4f}",
+                    f"{m['f1_naive']:.4f}",
+                ]
+            )
+
+    header = f"{'seq':>5} | {'frames':>6} | {'P':>6} {'R':>6} {'F1':>6}"
+    print(header)
+    print("-" * len(header))
+    for name, m in rows:
+        print(
+            f"{name:>5} | {m['num_frames']:>6d} | "
+            f"{m['precision']:6.2f} {m['recall']:6.2f} {m['f1']:6.2f}"
+        )
+    print(f"\nWritten {args.output_csv}")
+
+
+if __name__ == "__main__":
+    main()