docs: new acceptance bar (peer agreement, not fidelity) — and C1's documented fix is wrong
Jahni: "I do not need your work to be identical or near identical to what I had before, only having it 99.99% at worst reproducible if two people share the same seed, since everyone rebuilds it on multiplayer." Recorded as OPSTACK-PLAN 2.6.1, superseding 2.6's "recognisably the same place". Consequences, each recorded where it will be found: - C10 closed permanently rather than parked: it measures old-path vs new-path agreement and the two never coexist in a shipped world. - The equivalence tests keep their value as PORT-CORRECTNESS checks, not fidelity checks. Isosurface hard-fail stays; ULP grading is diagnostic only. - C9 promoted to top open risk. "Two people share a seed" is exactly what /fp:fast weakens across toolchains, and a Linux dedicated server against Windows clients compiles the density path under opposite float models. FPSemantics = Precise is the fix and the IWYU debt now blocks something that matters. - C1 unblocked: it was deferred only because it re-rolls the world's noise. Then, doing C1's arithmetic before applying its documented one-liner: THE FIX IS WRONG. It bounds SeedF but keeps the * 97.7 multiplier, so the coordinate term still reaches 1.6e6 where the ULP is 0.19 — 9.5x the ~0.02/voxel step. It would have left the bug live for mid-range seeds while closing the ticket. The real fix deletes the multipliers: they only decorrelate the ~40 noise sites, which is a hashing job. VoxelHash::SeedOffset(Seed, Site) gives a site-salted, bounded, final-units offset. Bounding SeedF alone would also funnel every seed through 16384 offsets shared by all sites; per-site salting requires a collision at all ~40 sites instead. The op stack has already inherited the bug via FSlabVoidSource, so it must land in both paths at once — and every further port copies it again. Docs only; the C1 fix is not written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+68
-2
@@ -75,6 +75,48 @@ Applied consistently at all `SeedF` definitions (`:682`, `:1756`, `:2066`, `:220
|
||||
|
||||
---
|
||||
|
||||
#### ⛔ CORRECTION 2026-07-27 — **the fix above is WRONG. Do not apply it.**
|
||||
|
||||
It bounds `SeedF` but leaves the **multiplier** in place, and the multiplier is where the magnitude
|
||||
comes from. `SeedF · 97.7` with `SeedF ≤ 16383` still reaches **1.6e6**, where the float ULP is:
|
||||
|
||||
| | max coord term | ULP there | vs the ~0.02/voxel coordinate step |
|
||||
|---|---|---|---|
|
||||
| `Seed = 1000` today | 9.8e4 | 0.012 | 0.6× — fine |
|
||||
| **audit's proposed fix, worst case** | **1.6e6** | **0.19** | **9.5× — still lattice-snaps** |
|
||||
|
||||
So the "fix" would have made the bug *less catastrophic* while leaving it live, and — worse —
|
||||
**closed the ticket.** It would have cost a build and a world re-roll to discover that mid-range
|
||||
seeds still degrade. (Found by doing the arithmetic before applying it, not after.)
|
||||
|
||||
**The actual fix: bound the OFFSET, not the seed — i.e. delete the multipliers.**
|
||||
|
||||
The `· 7.3f … · 97.7f` multipliers exist only to decorrelate the ~40 noise call sites from each
|
||||
other. That is a *hashing* job, and hashing does it better: give each site its own offset drawn from
|
||||
the full seed entropy, already in final units, with no multiplier to re-inflate it.
|
||||
|
||||
```cpp
|
||||
// VoxelHash — one definition, so the bound cannot drift per site.
|
||||
// Site-salted, final units, bounded: ULP at 16384 is 0.002 = 10% of a voxel step.
|
||||
FORCEINLINE float SeedOffset(int32 Seed, uint32 Site)
|
||||
{
|
||||
return (float)(Mix((uint32)Seed ^ (Site * 2654435761u)) & 0x3FFF);
|
||||
}
|
||||
```
|
||||
`WorldX * SF + SeedF * 83.1f` → `WorldX * SF + VoxelHash::SeedOffset(Seed, kSiteScallopX)`.
|
||||
|
||||
**Why this is strictly better, not just smaller:** bounding `SeedF` alone would squeeze all seeds
|
||||
through 16384 distinct noise offsets *shared across every site*, so two colliding seeds would give
|
||||
identical noise everywhere. Per-site salting means two seeds must collide at **all** ~40 sites to
|
||||
produce the same world — which is never.
|
||||
|
||||
**Scope:** ~40 call sites, mechanical, plus the same bug now inherited by the operator stack
|
||||
(`FSlabVoidSource::SeedF`, `VoxelDensityOpStack.cpp`). Must land in **both** paths in one change, or
|
||||
the equivalence tests will (correctly) scream. **Cost of delay: every archetype ported copies it
|
||||
again.**
|
||||
|
||||
---
|
||||
|
||||
### C2 — Per-chunk parameter caches have no layout key (stale after live-edit) ⚠️ **real, reproducible**
|
||||
|
||||
`VoxelGenerator.cpp:503-524`:
|
||||
@@ -247,7 +289,22 @@ reachable only with a very large origin room *and* short tunnels. Flagging it be
|
||||
|
||||
---
|
||||
|
||||
### C9 — The build uses `/fp:fast`, and the MP plan assumes bit-reproducible terrain ⚠️ **added 2026-07-27, measured not assumed**
|
||||
### C9 — The build uses `/fp:fast`, and the MP plan assumes bit-reproducible terrain 🔴 **NOW THE TOP OPEN RISK — promoted 2026-07-27**
|
||||
|
||||
> **Why this outranks everything else in this document now.** Jahni stated the actual acceptance bar:
|
||||
> *"only having it 99.99% at worst reproducible if two people share the same seed, since everyone
|
||||
> rebuilds it on multiplayer."* That is **precisely and only** the guarantee this entry says
|
||||
> `/fp:fast` weakens. Fidelity to the old world was dropped as a requirement (`§C10`); peer agreement
|
||||
> replaced it as the single bar. **This entry is now load-bearing.**
|
||||
>
|
||||
> **The concrete case to decide:** a **Linux dedicated server** generating collision geometry and nav
|
||||
> against **Windows clients** compiles the same density code under *opposite* float models
|
||||
> (`ClangToolChain` → precise, `VCToolChain` → `/fp:fast`). That is not a theoretical divergence.
|
||||
> Same-platform peers on the same build are fine.
|
||||
>
|
||||
> **The fix is known and blocked:** `FPSemantics = Precise` on this module, which needs the IWYU debt
|
||||
> cleared first (seven headers compile only via the shared PCH). That debt is now worth clearing on
|
||||
> *this* justification rather than on tidiness.
|
||||
|
||||
Found while chasing a 1-ULP difference between `GetMazeDensity` and its operator-stack port. The
|
||||
difference survived a full bisect down to code that is character-for-character identical, which
|
||||
@@ -332,7 +389,16 @@ risk is real. That is one build, and it settles it.
|
||||
|
||||
---
|
||||
|
||||
### C10 — The op-stack ULP residue: PARKED, with the evidence, 2026-07-27
|
||||
### C10 — The op-stack ULP residue: ~~PARKED~~ **CLOSED PERMANENTLY**, 2026-07-27
|
||||
|
||||
> **⛔ CLOSED, and now provably irrelevant — not merely parked.** Jahni, 2026-07-27: *"I do not need
|
||||
> your work to be identical or near identical to what I had before, only having it 99.99% at worst
|
||||
> reproducible if two people share the same seed."* This entry measures **old path vs new path**
|
||||
> agreement, and the two paths will never both exist in a shipped world. **No requirement anywhere
|
||||
> in the project depends on this number.** Do not spend another minute on it.
|
||||
>
|
||||
> What survives is the *method* lesson at the bottom of this entry, and the redirect: the
|
||||
> reproducibility that IS required is **`§C9`** — across binaries and platforms, not across refactors.
|
||||
|
||||
**Status: accepted and closed by decision (Jahni), not by explanation.** Do not reopen this without
|
||||
reading the whole entry — five hypotheses have already been measured and refuted, and re-deriving
|
||||
|
||||
Reference in New Issue
Block a user