twanghcmut's picture
|
download
raw
5.63 kB
# Cleanup rule
The rule this repo's modules were cleaned under. Paste it verbatim when asking for another file,
directory or package to be cleaned.
---
## The rule
> Clean this file. Refactor **architecture and prose only** — the input-to-output mathematical
> mapping must stay 100% identical.
>
> **1. Do not change behaviour.** No formula, hyper-parameter, dtype, RNG stream, operation order,
> log string or public name changes. If a cleanup would change a trained weight, a saved artifact
> or a logged line, do not do it — say so instead. Prove it afterwards: with docstrings stripped,
> the AST must be identical to before.
>
> **2. No essays.** A docstring is a one-line summary, then `Args:` / `Returns:` / `Raises:` /
> `Attributes:` — nothing else. At most one short paragraph of rationale, and only when it stops a
> real regression (below). Module docstrings say what the module is and where the design writeup
> lives; they do not restate it. No `WHY THIS SHAPE`-style banner sections, no lineage, no
> call-site inventories (they rot the moment a caller is added).
>
> **3. No reST markup.** No ``` ``x`` ```, no `` `x` ``, no `:class:`/`:meth:`/`:func:`/`:mod:`
> roles, no `#:` autodoc markers (a plain `#` says the same thing). Nothing renders any of it; it
> only makes the source harder to read. Write the plain name. A one-line `# ==== section name ====`
> divider is navigation and stays; a divider wrapping a paragraph of prose is an essay in disguise
> and goes.
>
> **4. Fields go in an `Attributes:` block.** Not an ASCII table, not a prose list, not trailing
> per-field comments — those drift out of alignment with the fields and nobody notices. Shapes and
> dtypes belong there: `q: [V, D] float64 joint configs.`
>
> **5. Comments are one or two lines.** Keep only what a reader could not derive from the code.
> Delete the rest.
>
> **6. Naming and structure.** Underscore names, no abbreviations that need a decoder. Prefer a
> class holding related state over parallel loose fields; prefer a frozen dataclass over a
> 10-field tuple or a `-1` sentinel. Split a function that answers more than one question — but
> only when the split is free of rule 1.
>
> **7. Do not churn.** A file already in this style is finished. Say so and move on. Editing it
> to produce a diff is worse than leaving it.
>
> When you are done, report: line counts before/after, what you kept and why, and the verification
> you ran.
---
## What counts as worth keeping
The test is: **would deleting this line let someone reintroduce a bug that the type system, the
tests and the code itself would not catch?** If yes, keep it — compressed to one line. Real
examples from this repo:
| kept | because |
|---|---|
| `off_manifold_distance` stays float64 numpy | pinned bit-exactly in `tests/golden/parity.json`; the float32 sibling drifts ~1e-6 |
| `_lodo_floors` replays cdist's own `\|a-b\|^2 = \|a\|^2 + \|b\|^2 - 2a.b` identity in float32 | any other algorithm drifts 1.3e-6 rad |
| `np.log`, not `math.log` | 1 ULP apart on some inputs, which is pass-vs-fail on an `assert_array_equal` |
| `weighted_topk` uses `torch.sort`, not `np.argsort` | they break ties differently, changing which nodes enter the support |
| the logsumexp stabiliser is per-destination, not per-relation | splitting it silently changes the aggregation semantics |
| `deg > 0` guard | a zero-degree node otherwise reaches a Linear as garbage and can NaN |
| `reached` is tracked explicitly, never inferred from `h == 0` | layer_norm can legitimately zero a reached node |
| `h^0` must be exactly zero off-seed | preserves the conditioning semantics the model was trained under |
| no parameter tensor has a dimension equal to `\|V\|` | the inductive guarantee |
| the two `owner`-numbering schemes in `field/build.py` | both are baked into artifacts on disk |
And what to delete without hesitation: design rationale that lives in `docs/`, module lineage,
"why this is its own module", performance budgets in FLOPs, lists of every current call site, and
any sentence explaining what the next line already says.
---
## Verification
Two gates, both mandatory.
**Code untouched.** Strip every docstring, compare the AST to the previous commit. Only rename or
deletion of a name should show up, and only if that was the intent:
```bash
python3 - <<'PY'
import ast, subprocess, pathlib
def strip(src):
t = ast.parse(src)
for n in ast.walk(t):
if isinstance(n, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)):
b = getattr(n, "body", None)
if b and isinstance(b[0], ast.Expr) and isinstance(b[0].value, ast.Constant) \
and isinstance(b[0].value.value, str):
n.body = b[1:] or [ast.Pass()]
return ast.dump(t)
bad = 0
for p in sorted(pathlib.Path("src").rglob("*.py")):
old = subprocess.run(["git", "show", f"HEAD:{p}"], capture_output=True, text=True)
if old.returncode:
continue
if strip(old.stdout) != strip(p.read_text()):
print("CODE CHANGED:", p); bad += 1
print("identical" if not bad else f"{bad} files changed code")
PY
```
**Tests.** `OMP_NUM_THREADS=4 PYTHONPATH=src python -m pytest tests/ -q`, and `tests/test_parity.py`
in particular — it is the bit-exactness net.
For rule 3, `scripts/strip_rest_markup.py` does the markup pass mechanically: it locates docstring
spans through the AST and comment spans through `tokenize`, so no code string literal is touched,
and it refuses to write any file whose docstring-stripped AST changed.
```bash
python3 scripts/strip_rest_markup.py src
```

Xet Storage Details

Size:
5.63 kB
·
Xet hash:
5edcf7f188b295850551ae8602cbcf0898eb79ea6f37389f6f0424d2abfdc481

Xet efficiently stores files, intelligently splitting them into unique chunks and accelerating uploads and downloads. More info.