Files
ConformalLabpp/doc/reviewer/usability-audit-2026-05-31.md
Tarik Moussa b8d6e23adf docs(contracts): fix three errors in processing-unit contract table (U3)
Finding-U3 from doc/reviewer/usability-audit-2026-05-31.md.

Three concrete errors corrected:

1. check_gauss_bonnet() row (was: implies all map types work)
   → now: 'EuclideanMaps/SphericalMaps only' — HyperIdealMaps overload
     is = delete (compile error since external-audit Finding-B fix)

2. enforce_gauss_bonnet() row (same issue)
   → same restriction note added

3. newton_hyper_ideal() preconditions (was: 'lambda0 initialised')
   → WRONG: HyperIdeal computes lengths from b_v, a_e via zeta functions
     internally; lambda0 plays no role.  Correct precondition:
     'no lambda0 needed · no GB pre-check'
   → Added explanation: strictly convex energy (Springborn 2020 Thm 1.3)
     → Newton converges for any targets without a pre-check

Gauss-Bonnet prose section also updated:
  - Examples now show EuclideanMaps/SphericalMaps explicitly
  - HyperIdeal compile-error note added with the correct hyperbolic
    identity Σ(2π-Θ_v) - Area = 2π·χ and why no pre-check is needed
  - Open-mesh note added (GB identity does not hold for boundary meshes)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 01:44:36 +02:00

20 KiB
Raw Blame History

Usability & Documentation Audit — ConformalLabpp v0.10.0

Date: 2026-05-31 Auditor: External reviewer (Claude Sonnet 4.6) Branch: review/usability-audit-2026-05-31 Base: main (post external-audit-2026-05-30 merge) Focus: Documentation quality, API usability, new-user experience

This document is self-contained. A new session can pick up any finding and act on it without prior context. Each finding includes exact file paths, the concrete problem, and a precise fix with acceptance criteria.

Status legend: 🔴 Usability bug · 🟡 Doc error/stale · 🟠 Missing content


How to read this document in a new session

git checkout review/usability-audit-2026-05-31

# No build needed for most fixes — documentation and example changes only.
# For changes that touch compilable code (examples), verify with:
cmake -S code -B build-cgal -DWITH_CGAL_TESTS=ON -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
cmake --build build-cgal --target conformallab_cgal_tests -j1
ctest --test-dir build-cgal -R '^cgal\.' --output-on-failure

FINDING-U1 — 🔴 CRITICAL: All examples show the trivial identity map, not real conformal flattening

Location

  • README.md lines 100121 ("Minimal usage")
  • code/examples/example_euclidean.cpp lines 7283 (Step 4)
  • code/examples/example_layout.cpp lines 4049 (set_natural_theta)
  • code/examples/example_hyper_ideal.cpp lines 8998 (Step 3)

Problem

Every example uses the "natural theta" trick:

auto G0 = euclidean_gradient(mesh, x0, maps);
for (auto v : mesh.vertices())
    if (maps.v_idx[v] >= 0)
        maps.theta_v[v] -= G0[maps.v_idx[v]];

This sets Θ_v to the actual angle sums at x=0, making x*=0 the equilibrium. The result: u_v ≈ 0 everywhere — no deformation. The map is identity.

Natural theta is a valid test pattern (it proves Newton converges in 0 or 1 steps and guards against solver regressions). But as the only pattern shown to a new user, it gives the false impression that the library produces trivial output. The real use case — conformally flatten a surface to the plane — is never demonstrated.

What is missing

A "real-world" example that shows the actual use case:

// Conformally flatten a mesh to the plane.
// Set Θ_v = 2π (regular interior vertex, zero cone angle defect) for all
// free vertices; enforce_gauss_bonnet makes the assignment topologically
// consistent.  The result is a flat conformal map with minimal angle distortion.
for (auto v : mesh.vertices())
    maps.theta_v[v] = conformallab::TWO_PI;
conformallab::enforce_gauss_bonnet(mesh, maps);
auto res = conformallab::newton_euclidean(mesh, x0, maps);
auto layout = conformallab::euclidean_layout(mesh, res.x, maps);
// layout.uv[v.idx()] now holds 2D coordinates of the flattening

Fix

  1. Add a new example code/examples/example_flatten.cpp:

    • Takes a real mesh from code/data/ (e.g. cathead.obj or torus_8x8.off)
    • Sets Θ_v = 2π, calls enforce_gauss_bonnet, solves Newton, computes layout
    • Saves a UV-mapped OFF file
    • Shows non-trivial u_v values at output
  2. Update README "Minimal usage" to show the flatten use case instead of (or in addition to) natural theta. Add a comment explaining what natural theta is and when to use it.

  3. Add a comment to all existing examples explaining that natural theta is a testing convenience, not a typical use case.

Acceptance criteria

  • example_flatten.cpp compiles and runs with a real mesh input
  • Output shows non-trivial u_v ≠ 0 and a visible UV parameterisation
  • README "Minimal usage" demonstrates real flattening (not identity)
  • Existing examples have a comment: "Note: natural theta → x* = 0 (testing pattern). For real flattening, set theta_v = TWO_PI and call enforce_gauss_bonnet."

FINDING-U2 — 🔴 CRITICAL: No example for the CGAL public API

Location

  • code/include/CGAL/Discrete_conformal_map.h (the "user-facing entry")
  • code/examples/ (no example uses CGAL::discrete_conformal_map_euclidean)

Problem

Discrete_conformal_map.h is documented as "User-facing entry point for the Discrete_conformal_map package." The Doxygen doc shows a minimal usage snippet:

auto result = CGAL::discrete_conformal_map_euclidean(mesh);

But this snippet only exists in a Doxygen comment — no compilable example demonstrates it. All four code/examples/*.cpp use the internal API (setup_euclidean_maps + newton_euclidean). A CGAL user who reaches this library via the CGAL ecosystem has no runnable starting point.

The CGAL API and the internal API also produce different result types:

  • Internal: NewtonResult.x — DOF-index-indexed std::vector<double>
  • CGAL: Conformal_map_result.u_per_vertex — vertex-index-indexed std::vector<FT>

This discrepancy is not explained anywhere for users.

Fix

Add code/examples/example_cgal_api.cpp:

// Demonstrates the CGAL public API (Discrete_conformal_map.h).
// Contrast with example_euclidean.cpp which uses the internal API directly.

#include <CGAL/Simple_cartesian.h>
#include <CGAL/Surface_mesh.h>
#include <CGAL/Discrete_conformal_map.h>
// ...
auto result = CGAL::discrete_conformal_map_euclidean(mesh);
// result.u_per_vertex[v.idx()] — per-vertex scale factor
// result.converged, result.iterations, result.gradient_norm

Also add to README: a short paragraph explaining when to use CGAL API vs internal API (CGAL API: simpler, less control; internal API: full pipeline, holonomy, period matrix).

Acceptance criteria

  • example_cgal_api.cpp compiles with -DWITH_CGAL=ON
  • Example uses CGAL::discrete_conformal_map_euclidean (not internal API)
  • README explains the two API levels and when to use each

FINDING-U3 — 🟡 DOC ERROR: contracts.md incorrect after Finding-B

Location

doc/api/contracts.md line 16

Problem

| `check_gauss_bonnet()` | `theta_v[v]` set for all vertices |
  Throws `std::runtime_error` if Σ(2πΘᵥ) ≠ 2π·χ(M) |

After the external-audit-2026-05-30 Finding-B fix, calling check_gauss_bonnet(mesh, hyper_ideal_maps) is a compile error (the overload is = delete). The contract table implies it works for all map types — which is now false.

Also missing from the table: the HyperIdeal functional needs no Gauss-Bonnet pre-check because its energy is strictly convex (Springborn 2020 Theorem 1.3) — Newton converges for any target angles.

Fix

| `check_gauss_bonnet()` | `theta_v[v]` set · **Euclidean or Spherical maps only** |
  Throws if Σ(2πΘᵥ) ≠ 2π·χ(M). **Not applicable to HyperIdealMaps** —
  deleted overload; HyperIdeal needs no pre-check (strictly convex energy). |

Also add a row for enforce_gauss_bonnet:

| `enforce_gauss_bonnet()` | `theta_v[v]` set · **Euclidean or Spherical maps only** |
  Shifts all Θᵥ by δ = (lhsrhs)/V so that GB holds exactly.
  **Not applicable to HyperIdealMaps** — deleted overload. |

Acceptance criteria

  • check_gauss_bonnet row notes "Euclidean/Spherical only, not HyperIdeal"
  • enforce_gauss_bonnet row added with same note
  • A brief explanation why HyperIdeal doesn't need GB: "strictly convex energy"

FINDING-U4 — 🟡 DOC ERROR: Stale version in README (v0.9.0)

Location

README.md line 17

Problem

**Status:** v0.9.0 — Phases 19a complete, Phase 8b-Lite CGAL API surface.

The current release is v0.10.0 (tagged on main; CLAUDE.md confirms this). CLAUDE.md was updated to v0.10.0 but the README was not.

Fix

**Status:** v0.10.0 — Phases 19b complete. Newton solvers for five DCE
models (Euclidean / Spherical / HyperIdeal / CP-Euclidean / Inversive-Distance),
priority-BFS layout in ℝ²/S²/Poincaré disk, GaussBonnet, tree-cotree cut
graph, Möbius holonomy, period matrix (genus 1), fundamental domain,
halfedge_uv texture atlas, JSON/XML serialisation, CLI app.
277 tests passing, 0 skipped — see [`doc/api/tests.md`](doc/api/tests.md).

Acceptance criteria

  • README status line shows v0.10.0 and the correct feature list
  • Test count updated to 277

FINDING-U5 — 🟡 DOC ERROR: CGAL header comment describes superseded state

Location

code/include/CGAL/Discrete_conformal_map.h lines 1416

Problem

/// This header provides a single function — `discrete_conformal_map_euclidean`
/// — that computes a Euclidean discrete-conformal flattening … Spherical and
/// hyperbolic variants are scheduled for Phase 8b.2 once the Euclidean
/// pattern is validated

But the file already contains discrete_conformal_map_spherical() and discrete_conformal_map_hyper_ideal() (and the other models). The comment describes Phase 8a state; the file is at Phase 8b-Lite.

Fix

Update the \file Doxygen block at the top of Discrete_conformal_map.h:

/*!
\file CGAL/Discrete_conformal_map.h
\ingroup PkgConformalMapRef

User-facing entry points for the Discrete_conformal_map package.

This header provides five functions covering all three DCE geometries:
  - `CGAL::discrete_conformal_map_euclidean`   (flat / ℝ²)
  - `CGAL::discrete_conformal_map_spherical`   (S², genus 0)
  - `CGAL::discrete_conformal_map_hyper_ideal` (H², genus ≥ 1)
  - `CGAL::discrete_circle_packing_euclidean`  → Discrete_circle_packing.h
  - `CGAL::discrete_inversive_distance_map`    → Discrete_inversive_distance.h
...
*/

Acceptance criteria

  • \file doc block lists all five entry functions
  • No forward-reference to "planned Phase 8b.2" — it is already implemented

FINDING-U6 — 🟡 STALE EXAMPLES: New gauge-vertex overload not reflected

Location

  • README.md lines 107110
  • code/examples/example_euclidean.cpp lines 6268
  • code/examples/example_layout.cpp lines 5259 (pin_first helper)

Problem

Finding-D (external-audit-2026-05-30) added a clean gauge-vertex overload:

assign_euclidean_vertex_dof_indices(mesh, maps, gauge_vertex);

But all examples still use the old verbose loop:

auto vit = mesh.vertices().begin();
maps.v_idx[*vit++] = -1;   // pinned
int idx = 0;
for (; vit != mesh.vertices().end(); ++vit)
    maps.v_idx[*vit] = idx++;

A new user copying from the examples will write the old error-prone pattern instead of the new clean overload — defeating the purpose of the fix.

Fix

Replace the old loop in all three locations with the new overload:

// Pin the first vertex, assign sequential DOF indices to the rest.
int n = assign_euclidean_vertex_dof_indices(mesh, maps,
                                            *mesh.vertices().begin());

Same for assign_vertex_dof_indices (spherical) and assign_inversive_distance_vertex_dof_indices wherever they appear in examples.

Acceptance criteria

  • README "Minimal usage" uses the overload
  • example_euclidean.cpp uses the overload
  • example_layout.cpp pin_first helper replaced with the overload
  • Old manual loop removed from all user-facing code

FINDING-U7 — 🟠 MISSING: CONFORMALLAB_LOW_MEMORY_BUILD absent from getting-started.md

Location

doc/getting-started.md — "Build modes" section

Problem

getting-started.md documents five compile-time modes but does not list CONFORMALLAB_LOW_MEMORY_BUILD=ON. A Raspberry Pi or low-RAM user who reads getting-started.md (the natural first stop) cannot find the flag. It only appears in CLAUDE.md and the README table.

Fix

Add to getting-started.md after the CONFORMALLAB_FAST_TEST_BUILD entry:

### Low-memory build (RAM-constrained CI / Raspberry Pi)

For machines with ≤ 4 GB RAM, the default CGAL build (PCH + -O3) OOMs
the compiler.  `LOW_MEMORY_BUILD` uses `-O0`, disables PCH, and sets
unity batch size to 1, keeping `cc1plus` peak at ~150-200 MB per TU:

```bash
cmake -S code -B build -DWITH_CGAL_TESTS=ON \
      -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
cmake --build build --target conformallab_cgal_tests -j1

Tests run ~15× slower at -O0 but all pass. Useful for CI runners with limited memory.


### Acceptance criteria
- [ ] `getting-started.md` documents `CONFORMALLAB_LOW_MEMORY_BUILD`
- [ ] Example command shows `-j1` (required for memory safety)

---

## FINDING-U8 — 🟠 MISSING: `LOW_MEMORY_BUILD` absent from README compile-time table

### Location
`README.md` lines 5382 ("Compile-time workflow modes")

### Problem

The README code block shows five compile-time patterns but omits
`CONFORMALLAB_LOW_MEMORY_BUILD`.  This is the flag that re-enabled CI
and is directly relevant to anyone building on resource-constrained hardware.

### Fix

Add to the README compile-time examples:
```bash
# Low-memory build: -O0, no PCH, unity batch 1 (~150 MB peak per cc1plus).
# Use on Raspberry Pi / CI runners with ≤ 4 GB RAM.  Tests run ~15× slower.
cmake -S code -B build -DWITH_CGAL_TESTS=ON -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
cmake --build build --target conformallab_cgal_tests -j1

Acceptance criteria

  • README shows LOW_MEMORY_BUILD usage with a one-line explanation

FINDING-U9 — 🟠 MISSING: layout.uv indexing semantics not documented

Location

code/examples/example_layout.cpp line 127 code/include/layout.hppLayout2D struct definition

Problem

auto& p = layout.uv[v.idx()];  // example_layout.cpp:127

Layout2D.uv is a std::vector indexed by the raw integer .idx() of Vertex_index. This is undocumented: nowhere is it stated that the vector length equals num_vertices(mesh) and that the index is contiguous. In CGAL, vertex.idx() is contiguous on a fresh Surface_mesh but may have gaps after vertex removals.

The halfedge_uv field (also in Layout2D) has the same issue for halfedge indices.

Fix

In layout.hpp, add explicit documentation to the Layout2D struct:

struct Layout2D {
    /// UV coordinate of vertex v in the layout plane.
    /// Indexed by `v.idx()` — length equals `mesh.number_of_vertices()`.
    /// \pre  No vertices have been removed from `mesh` since construction
    ///       (i.e., `mesh.is_valid(false)` and no compaction was performed).
    std::vector<Eigen::Vector2d> uv;

    /// UV of source(h) as seen from face(h), indexed by `h.idx()`.
    /// At seam halfedges the two opposite halfedges carry different UV values,
    /// enabling proper per-halfedge UV for GPU texture atlasing.
    std::vector<Eigen::Vector2d> halfedge_uv;
    ...
};

Also update example_layout.cpp to add a comment:

// layout.uv is indexed by v.idx() — valid as long as no vertices were
// removed from the mesh after loading.
auto& p = layout.uv[v.idx()];

Acceptance criteria

  • Layout2D.uv and Layout2D.halfedge_uv have explicit Doxygen docs stating the index semantics and the no-compaction precondition
  • example_layout.cpp has an inline comment at the access site

FINDING-U10 — 🟠 MISSING: CLI parameter reference not in README or getting-started

Location

  • README.md (has CLI examples but no parameter table)
  • doc/getting-started.md (same)
  • code/src/apps/v0/conformallab_cli.cpp lines 333340 (source of truth)

Problem

The CLI app (conformallab_core) is the primary non-programmatic entry point for the library. The README shows a few usage examples but no parameter reference. A user running --help gets CLI11's auto-generated output, but that output is not reproduced anywhere in the documentation.

Current parameters (from conformallab_cli.cpp):

Flag Default Description
-i / --input required Input mesh (OFF / OBJ / PLY)
-o / --output (none) Output layout OFF file
-j / --json (none) Serialise result to JSON
-x / --xml (none) Serialise result to XML
-g / --geometry euclidean Target geometry: euclidean | spherical | hyper_ideal
-s / --show false Open input mesh in interactive viewer
-v / --verbose false Print topology / DOF stats

Fix

Add a concise parameter table to doc/getting-started.md in the "First run — CLI app" section. The README already shows the most common invocations; the table can live in getting-started to avoid README bloat.

Acceptance criteria

  • doc/getting-started.md has a parameter table matching the source
  • Each flag has a one-line description and notes the default
  • Cross-reference to --help for the canonical up-to-date list

FINDING-U11 — 🟠 ROADMAP: CLI missing three useful parameters + two models

Location

code/src/apps/v0/conformallab_cli.cpp

Problem (not a bug — a gap for a future session to fill)

Missing solver-tuning parameters (low effort, high value):

The Newton tolerance and iteration limit are hardcoded at their defaults (tol=1e-8, max_iter=200). Any user who wants to solve a stiff mesh more carefully or stop early has to recompile.

# Not yet possible:
./conformallab_core -i hard_mesh.off -g euclidean --tol 1e-12 --max-iter 1000
./conformallab_core -i quick_check.off --tol 1e-4 --max-iter 20

Missing geometry modes (medium effort):

The two Phase-9a functionals are not reachable from the CLI at all:

# Not yet possible — needs run_cp_euclidean() + run_inversive_distance() helpers:
./conformallab_core -i mesh.off -g cp_euclidean
./conformallab_core -i mesh.off -g inversive_distance

CP-Euclidean (Discrete_circle_packing.h) and Inversive-Distance (Discrete_inversive_distance.h) are fully implemented in the library and exposed via the CGAL public API, but a CLI user cannot reach them.

Suggested implementation (for a future session)

  1. --tol <double> and --max-iter <int>: one app.add_option each, thread through all three run_* functions. ~15 lines.

  2. -g cp_euclidean and -g inversive_distance: add two new run_cp_euclidean() / run_inversive_distance() helpers following the existing run_euclidean() pattern. ~60 lines each. Add to the -g CLI::IsMember list. Register the two new geometry strings in the dispatch block.

  3. Update README + getting-started to list all five geometry modes.

Effort estimate

  • --tol + --max-iter: ~30 min
  • Phase-9a CLI exposure: ~2 hours (mostly copy-paste + adaptation)
  • Tests: add two CLI smoke tests (geometry converges on tetrahedron)

Acceptance criteria (when implemented)

  • --tol and --max-iter are accepted and forwarded to all three/five solvers
  • ./conformallab_core -i mesh.off -g cp_euclidean -o out.off works
  • ./conformallab_core -i mesh.off -g inversive_distance -o out.off works
  • --help output lists all five geometry modes

Summary table

ID File Type Severity Status
U1 README + 4 examples Usability 🔴 Critical Fixed 2026-05-31
U2 examples/ (missing) Usability 🔴 Critical Fixed 2026-05-31
U3 contracts.md:16 Doc error 🟡 Medium Fixed 2026-05-31
U4 README.md:17 Stale 🟡 Medium 🟡 Open
U5 Discrete_conformal_map.h:14 Stale 🟡 Medium 🟡 Open
U6 README + 2 examples Stale 🟡 Medium 🟡 Open
U7 getting-started.md Missing 🟠 Minor 🟡 Open
U8 README.md Missing 🟠 Minor 🟡 Open
U9 layout.hpp + example Missing 🟠 Minor 🟡 Open
U10 getting-started.md Missing 🟠 Minor 🟡 Open
U11 conformallab_cli.cpp Roadmap 🟠 Minor 🟡 Open

Priority order for fixing:

  1. U1 + U2 done
  2. U3 (correctness — contracts.md wrong after Finding-B)
  3. U4 + U5 + U6 (stale content — quick fixes)
  4. U7 + U8 + U9 + U10 (missing docs — minor)
  5. U11 (roadmap — CLI extensions, not urgent)