7a1686daa5
Some checks failed
C++ Tests / test-fast (pull_request) Successful in 2m26s
API Docs / doc-build (pull_request) Successful in 51s
Markdown link check / check (pull_request) Successful in 48s
C++ Tests / test-cgal (pull_request) Failing after 11m25s
C++ Tests / quality-gates (pull_request) Successful in 2m22s
Three reviewer-meeting deliverables in one commit.
(1) output_uv_map for the two remaining DCE entries
─────────────────────────────────────────────────
* Discrete_inversive_distance.h: full implementation. After Newton,
reconstruct effective Euclidean edge lengths from the converged
log-radii via the Bowers-Stephenson identity
`ℓᵢⱼ² = rᵢ² + rⱼ² + 2·Iᵢⱼ·rᵢ·rⱼ`, populate a temporary
EuclideanMaps with `lambda0 = log(ℓᵢⱼ²)`, and reuse the existing
`euclidean_layout(mesh, 0, eucl)` priority-BFS. Per-vertex
Point_2 coordinates written into the user-supplied pmap.
Optional `normalise_layout(true)` applies the canonical PCA
centroid + major-axis rotation, same as the other 3 entries.
* Discrete_circle_packing.h: throws std::runtime_error with a
clear pointer to Phase 9c rather than silently producing
nonsense. CP-Euclidean is face-based; the faithful output is a
per-face circle packing in ℝ², not a per-vertex Point_2 map.
A true layout requires BPS-2010 §6 (~150 lines, on the porting
roadmap as Phase 9c). Failing loudly is the honest default.
Tests: 2 new cases in test_cgal_phase8b_lite.cpp
(OutputUvMap_InversiveDistance_PopulatesPmap;
OutputUvMap_CPEuclidean_ThrowsClearly). Both green.
Suite total now 259 (was 257, +2). CGAL subtotal: 234 → 236.
(2) Reviewer meeting documents
──────────────────────────
New directory doc/reviewer/ with three files:
* briefing.md — one-page orientation for the reviewer.
What the project is, where to look first
(https://tmoussa.codeberg.page/ConformalLabpp/), the headline
evidence (tests/coverage/sanitizers/license), what we want from
them, what's deferred and why, and the 5 questions in a separate
file.
* questions.md — the 5 concrete decisions we want their second
opinion on:
Q1 Phase 9c (port-literal vs re-derive)
Q2 Phase 9b-analytic (worth ~2 weeks for ~6× speedup?)
Q3 CP-Euclidean output_uv_map (build now or defer?)
Q4 CGAL submission strategy (one package or five?)
Q5 geometry-central cross-validation co-authorship
Plus an explicit "what would you say no to?" question at the
bottom — negative feedback is the highest-value information.
* agenda.md — my own internal playbook (NOT to be sent).
60-min flow: 5-min thank-you, 10-min architecture tour,
30-min for Q1-Q5 in the order Q4-Q1-Q2-Q5-Q3, 5-min "no"
question, 5-min wrap-up. Includes post-meeting memo template
to fill out in the 30 min after.
* README.md — index for the directory; says which file goes
to whom and when to send.
(3) locked-vs-flexible.md known-limitations update
─────────────────────────────────────────────
"output_uv_map covers 3 of 5 entries" → "covers 4 of 5".
CP-Euclidean's throws-clearly behaviour documented as a Phase 9c
deliverable rather than a passive gap.
Bonus: extended .codespellrc ignore list (acknowledgement, the
British-English spelling I used in agenda.md).
Verifications on this commit:
259/259 tests pass (0 skipped)
scripts/check-test-counts.sh: OK (23 + 236 = 259)
scripts/quality/license-headers.sh: OK (66/66 SPDX)
python3 scripts/quality/cgal-conventions.py: OK (0/6 violations)
scripts/quality/codespell.sh: OK (0 typos)
scripts/quality/shellcheck.sh: OK (0 findings)
python3 scripts/check-markdown-links.py: OK (143/143)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
113 lines
5.3 KiB
Markdown
113 lines
5.3 KiB
Markdown
# Reviewer briefing — conformallab++
|
||
|
||
> **Audience.** External reviewer with a discrete-conformal-geometry
|
||
> background (Springborn / Bobenko alumnus, ~30 minutes lead-time
|
||
> before the meeting).
|
||
>
|
||
> **Purpose of this document.** A single page they can read once
|
||
> before the meeting and feel oriented — what the project is, what
|
||
> we want from them, and where to look first.
|
||
|
||
## In one paragraph
|
||
|
||
`conformallab++` is a C++17 header-only re-implementation of the Java
|
||
library [`ConformalLab`](https://github.com/varylab/conformallab)
|
||
(Sechelmann 2016, TU Berlin), built around CGAL's `Surface_mesh` and
|
||
Eigen. v0.9.0 ships five Discrete Conformal Equivalence (DCE) solvers
|
||
— Euclidean, Spherical, HyperIdeal, Circle-Packing Euclidean
|
||
(BPS 2010, face-based), Inversive-Distance (Luo 2004, vertex-based) —
|
||
plus the Newton infrastructure, layout (priority-BFS trilateration in
|
||
ℝ², S², Poincaré disk), Möbius holonomy, period matrix, cut graph, and
|
||
JSON/XML serialisation. Long-term goal: a CGAL package.
|
||
|
||
## Where to start (one URL, 5 minutes)
|
||
|
||
**👉 https://tmoussa.codeberg.page/ConformalLabpp/**
|
||
|
||
The landing page is a hand-curated reviewer hub, not an auto-generated
|
||
index. It links to the Doxygen API, the key markdown documents, and
|
||
shows static quality-gate status.
|
||
|
||
## What's true about this snapshot
|
||
|
||
| Claim | Concrete evidence |
|
||
|---|---|
|
||
| **Library is header-only and standalone** | Verification recipe in `doc/architecture/dependencies.md`: `env -i PATH=… cmake … && ctest` passes with zero quality tools installed. |
|
||
| **Tests: 259 pass, 0 skipped** | `bash scripts/check-test-counts.sh` enforces this against `doc/api/tests.md`; CI fails on drift. |
|
||
| **Doxygen: 100 % public-API coverage, 0 warnings** | `bash scripts/doxygen-coverage.sh --threshold 100` is in CI. |
|
||
| **License hygiene: 66/66 files carry MIT SPDX** | `bash scripts/quality/license-headers.sh` is in CI (strict). |
|
||
| **Build reproducibility: byte-identical between runs** | `bash scripts/quality/reproducible-build.sh` (local, ~6 min). |
|
||
| **Sanitizers (ASan + UBSan) clean on fast suite** | `bash scripts/quality/sanitizers.sh` (local, ~3 min). |
|
||
| **CGAL conventions: 6 rules, 0 violations** | `python3 scripts/quality/cgal-conventions.py` (CI required). |
|
||
|
||
## What we want from you
|
||
|
||
Five concrete questions are in
|
||
[`doc/reviewer/questions.md`](questions.md) — please skim them
|
||
beforehand. They are deliberately scoped: each can be answered with
|
||
"go this way" / "no, go that way" / "either is fine".
|
||
|
||
The high-stakes ones:
|
||
|
||
1. **Phase 9c (4g-polygon)** — port the Java implementation literally,
|
||
or re-derive from Springborn 2020 §5?
|
||
2. **Phase 9b-analytic** — is the ~6× speedup over our current block-FD
|
||
Hessian (full Schläfli-based analytic) worth ~2 weeks? Working-mesh
|
||
size that justifies it?
|
||
3. **CGAL submission strategy** — submit as a single package or split
|
||
the five DCE solvers into separate packages?
|
||
|
||
## What's deliberately deferred (so we can discuss with you first)
|
||
|
||
| Item | Why deferred |
|
||
|---|---|
|
||
| Phase 9b-analytic (Schläfli-based HyperIdeal Hessian) | derivation done (805-line LaTeX doc), implementation depends on your opinion of payoff |
|
||
| Phase 9c (fundamental-polygon utility, 4g-polygon canonical form) | algorithm choice up to you |
|
||
| Cross-validation against geometry-central (GC-1) | potential paper, scope depends on your interest |
|
||
| CP-Euclidean `output_uv_map` (per-face circle packing) | needs the BPS-2010 §6 layout algorithm, ~3 days |
|
||
| `.a().b().c()` member-style named-parameter chaining | requires patching CGAL upstream; pipe-operator (`a | b | c`) shipped instead |
|
||
|
||
These are all flagged in
|
||
[`doc/architecture/locked-vs-flexible.md`](../architecture/locked-vs-flexible.md)
|
||
§"Known limitations".
|
||
|
||
## Architectural decisions you might want to challenge
|
||
|
||
12 decisions classified 🔴 load-bearing / 🟡 semi-fixed / 🟢 opportunistic
|
||
in [`doc/architecture/locked-vs-flexible.md`](../architecture/locked-vs-flexible.md).
|
||
The ones most worth your time:
|
||
|
||
- **#1 Surface_mesh as default** — 🔴 ~3 weeks to change. Are you OK
|
||
with this default, or should we wire Polyhedron_3 / OpenMesh now?
|
||
- **#6 Eigen as linear-algebra back-end** — 🔴 ~2 weeks to change. Are
|
||
the Eigen sparse solvers (SparseCholesky + SparseQR fallback)
|
||
sufficient for the mesh sizes you've seen, or should we look at
|
||
CHOLMOD / PETSc?
|
||
- **#7 Strategy C** (one Default trait per functional, not a unified
|
||
trait) — 🟡 ~1 week to refactor. CGAL convention agrees; do you?
|
||
|
||
## How to actually run something
|
||
|
||
```bash
|
||
git clone https://codeberg.org/TMoussa/ConformalLabpp && cd ConformalLabpp
|
||
cmake -S code -B build && cmake --build build --target conformallab_tests
|
||
ctest --test-dir build # ~2 s, 23 pure-math tests
|
||
# CGAL tests (adds Boost as a system dep):
|
||
cmake -S code -B build -DWITH_CGAL_TESTS=ON
|
||
cmake --build build --target conformallab_cgal_tests -j
|
||
ctest --test-dir build # ~3 min, 236 CGAL tests
|
||
```
|
||
|
||
A more end-to-end recipe lives in `scripts/try_it.sh` (also run in CI).
|
||
|
||
## Meeting logistics
|
||
|
||
- **Format**: video call (you suggested), ~60 min
|
||
- **Materials needed on your side**: just a browser to follow the
|
||
reviewer-hub URL.
|
||
- **Materials I'll have ready**: a screen-share-able terminal with
|
||
the repo open, my own agenda in `doc/reviewer/agenda.md`, and the
|
||
questions doc above.
|
||
|
||
Looking forward.
|