diff --git a/manifold/public/nisps.wasm b/manifold/public/nisps.wasm index 9a15915..1ceed31 100755 Binary files a/manifold/public/nisps.wasm and b/manifold/public/nisps.wasm differ diff --git a/manifold/src/engine/dataset.ts b/manifold/src/engine/dataset.ts index c8e6323..8abb0d9 100644 --- a/manifold/src/engine/dataset.ts +++ b/manifold/src/engine/dataset.ts @@ -42,7 +42,19 @@ export class Dataset { private labels_: Float32Array = new Float32Array(0); private size_ = 0; - constructor(maxSize = 100) { + /** + * `maxSize` has NO default on purpose: this store mirrors the C++ MLP's + * example ring buffer 1:1 (FIFO eviction order, sample-weight indexing), + * so its cap must always come from the C++ side's actual `max_examples()` + * (via `nisps_ml_describe`) rather than a value picked independently here. + * A default invited exactly that divergence — see S35 + * (docs/specs/recon/simplification-audit-2026-07.md): the JS mirror + * defaulted to 100 while the C++ ring defaulted to 128, so train() and + * trainAsync() silently trained on different data past 100 examples, and + * sample-weight buffers sized to the JS side's count read out of bounds + * on the C++ side once its count exceeded that. + */ + constructor(maxSize: number) { if (maxSize <= 0) throw new Error('Dataset.maxSize must be > 0'); this.maxSize = maxSize; } diff --git a/manifold/src/engine/types.ts b/manifold/src/engine/types.ts index b795129..ea180c0 100644 --- a/manifold/src/engine/types.ts +++ b/manifold/src/engine/types.ts @@ -56,7 +56,11 @@ export interface NispsModule { _nisps_ml_set_weights(ml: number, in_ptr: number): void; _nisps_ml_draw_weights(ml: number, spread: number): void; _nisps_ml_get_layer_stats(ml: number, out_ptr: number): void; - // Null ml reports the DEFAULT shape; a handle reports its runtime shape. + // Writes 7 ints: [in, h1, h2, h3, out, n_layers, max_examples]. Null ml + // reports the DEFAULT shape; a handle reports its runtime shape. + // max_examples is the C++ example-store ring-buffer capacity — the JS + // `Dataset` mirror MUST be sized to this value, not a hardcoded literal + // (see S35, docs/specs/recon/simplification-audit-2026-07.md). _nisps_ml_describe(ml: number, out_ptr: number): void; // ML feedback — the "Down Action" state machine (Avoid / RandomiseOutputs / @@ -169,6 +173,8 @@ export interface MLArchitecture { hidden: [number, number, number]; outputSize: number; numLayers: number; + /** C++ example-store ring-buffer capacity — sizes the JS `Dataset` mirror. */ + maxExamples: number; } /** Per-layer weight health record (one per layer). */ diff --git a/manifold/src/engine/wasm-iml.ts b/manifold/src/engine/wasm-iml.ts index 79e749d..df46160 100644 --- a/manifold/src/engine/wasm-iml.ts +++ b/manifold/src/engine/wasm-iml.ts @@ -41,6 +41,14 @@ import { createTrainer, type WasmTrainer } from './wasm-worker'; /** Default architecture matches `nisps/wasm/bindings.cpp` instantiation. */ const DEFAULT_INPUT_SIZE = 2; const DEFAULT_OUTPUT_SIZE = 126; +/** + * Placeholder only — overwritten by `init_()` with the live value read from + * `nisps_ml_describe` (out_dims[6]). Matches `nisps::ml::kDefaultMaxExamples` + * (nisps/ml/storage.hpp), the single source of truth for the C++ example- + * store ring-buffer capacity. Do NOT hardcode a different number for the JS + * `Dataset` mirror's cap — see S35 (docs/specs/recon/simplification-audit-2026-07.md). + */ +const DEFAULT_MAX_EXAMPLES = 128; /** Base-aware absolute URL for an asset served from `public/`. Resolves against * `document.baseURI` (the page URL) so a `base: './'` build works under any @@ -116,6 +124,15 @@ export interface WasmIMLOptions { seed?: number; /** localStorage key the loaded weights/dataset will be persisted under. */ storageKey?: string; + /** + * Currently a no-op: `nisps_ml_create` has no max_examples parameter, so + * the C++ example-store ring buffer is always sized to + * `nisps::ml::kDefaultMaxExamples`, and the JS `Dataset` mirror must match + * it exactly (S35) — there is no way to honour a caller-supplied override + * without also plumbing one through the C API. Kept on the options type so + * a future create()-side max_examples parameter has somewhere to land; + * unused today. + */ maxExamples?: number; /** Injected side-effect boundary. Defaults to a no-op sink (headless use). */ sink?: EngineSink; @@ -131,6 +148,7 @@ export class WasmIML { hidden: [10, 14, 18], outputSize: DEFAULT_OUTPUT_SIZE, numLayers: 4, + maxExamples: DEFAULT_MAX_EXAMPLES, }; private featuresBuf!: HeapBuffer; @@ -153,7 +171,13 @@ export class WasmIML { private curveBuf!: HeapBuffer; // curve batch scratch (chunked) private static CURVE_CHUNK = 256; - readonly dataset: Dataset; + // Constructed in init_(), once the live max_examples() is known from + // nisps_ml_describe — NOT in the constructor, which runs before the WASM + // module is loaded. Sizing this any other way (e.g. a hardcoded literal) + // is exactly the S35 bug: it must always match the C++ ring buffer's + // actual capacity or train()/trainAsync() silently diverge and + // sample-weight buffers sized to it read out of bounds C++-side. + dataset!: Dataset; private readonly sink: EngineSink; private lastLoss_: number | null = null; private trainer: WasmTrainer | null = null; @@ -164,7 +188,6 @@ export class WasmIML { static MAX_BATCH = 4096; private constructor(opts: WasmIMLOptions) { - this.dataset = new Dataset(opts.maxExamples ?? 100); this.storageKey = opts.storageKey ?? 'nisps:wasm-iml'; this.sink = opts.sink ?? noopSink; } @@ -184,9 +207,10 @@ export class WasmIML { // Default shape (null handle). Since one-core-engine P2 the MLP is // runtime-shaped: create() honours requested dims; we pass the caller's // sizes (falling back to the defaults) and re-describe the instance. - this.describePtr = this.module._malloc(6 * 4); + // 7 ints: [in, h1, h2, h3, out, n_layers, max_examples] (S35). + this.describePtr = this.module._malloc(7 * 4); this.module._nisps_ml_describe(0, this.describePtr); - const defaults = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 6); + const defaults = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 7); const wantedIn = opts.inputSize ?? defaults[0]; const wantedOut = opts.outputSize ?? defaults[4]; @@ -195,14 +219,19 @@ export class WasmIML { if (!this.mlHandle) throw new Error('[wasm-iml] nisps_ml_create returned null'); this.module._nisps_ml_describe(this.mlHandle, this.describePtr); - const dims = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 6); + const dims = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 7); this.arch_ = { inputSize: dims[0], hidden: [dims[1], dims[2], dims[3]], outputSize: dims[4], numLayers: dims[5], + maxExamples: dims[6], }; + // The JS Dataset mirror's cap MUST equal the C++ ring buffer's actual + // max_examples() — read from describe(), never hardcoded (S35). + this.dataset = new Dataset(this.arch_.maxExamples); + this.weightCount_ = this.module._nisps_ml_weight_count(this.mlHandle); this.featuresBuf = new HeapBuffer(this.module, this.arch_.inputSize); @@ -327,14 +356,19 @@ export class WasmIML { // Re-describe the (new) instance and refresh the weight count. this.module._nisps_ml_describe(this.mlHandle, this.describePtr); - const d = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 6); + const d = new Int32Array(this.module.HEAP32.buffer, this.describePtr, 7); this.arch_ = { inputSize: d[0], hidden: [d[1], d[2], d[3]], outputSize: d[4], numLayers: d[5], + maxExamples: d[6], }; this.weightCount_ = this.module._nisps_ml_weight_count(this.mlHandle); + // nisps_ml_reshape never varies max_examples (no such parameter exists on + // that C API) — it always reconstructs at kDefaultMaxExamples, same as + // create(). The Dataset mirror's cap is therefore still correct; only its + // contents are cleared below, matching the C++ side's dataset reset. // Reallocate every dim-dependent heap buffer. Freeing first then reallocating // means a later malloc may sbrk-grow the heap and detach earlier views, so we diff --git a/manifold/src/engine/wasm-worker.ts b/manifold/src/engine/wasm-worker.ts index 097afb3..0a66faa 100644 --- a/manifold/src/engine/wasm-worker.ts +++ b/manifold/src/engine/wasm-worker.ts @@ -212,9 +212,15 @@ if (isWorker) { workerSeed = seed >>> 0; mlHandle = mod._nisps_ml_create(0, 0, 0, 0, workerSeed); weightCount = mod._nisps_ml_weight_count(mlHandle); - const dPtr = mod._malloc(6 * 4); + // 7 ints: [in, h1, h2, h3, out, n_layers, max_examples] — the buffer size + // and view length here MUST track nisps_ml_describe's actual output + // (nisps/wasm/bindings.cpp) or this overflows the WASM heap by 4 bytes + // (S35, docs/specs/recon/simplification-audit-2026-07.md). This worker + // doesn't use max_examples (it never touches the dataset), but describe() + // always writes all 7 regardless of what the caller reads. + const dPtr = mod._malloc(7 * 4); mod._nisps_ml_describe(mlHandle, dPtr); - const d = new Int32Array(mod.HEAP32.buffer, dPtr, 6); + const d = new Int32Array(mod.HEAP32.buffer, dPtr, 7); netInputSize = d[0]; netOutputSize = d[4]; netHidden = [d[1], d[2], d[3]]; diff --git a/nisps/CMakeLists.txt b/nisps/CMakeLists.txt index fed17bd..a1a9687 100644 --- a/nisps/CMakeLists.txt +++ b/nisps/CMakeLists.txt @@ -56,6 +56,7 @@ if(NOT EMSCRIPTEN) ${NISPS_TEST_DIR}/test_mlp_loss.cpp ${NISPS_TEST_DIR}/test_mlp_rl.cpp ${NISPS_TEST_DIR}/test_mlp_storage_parity.cpp + ${NISPS_TEST_DIR}/test_mlp_storage_defaults.cpp ${NISPS_TEST_DIR}/test_mlp_jolt.cpp ${NISPS_TEST_DIR}/test_mlp_ou_noise.cpp ${NISPS_TEST_DIR}/test_mlp_feedback.cpp diff --git a/nisps/ml/dynamic_storage.hpp b/nisps/ml/dynamic_storage.hpp index 5aa485f..942c377 100644 --- a/nisps/ml/dynamic_storage.hpp +++ b/nisps/ml/dynamic_storage.hpp @@ -27,7 +27,7 @@ #include #include -#include "storage.hpp" // kMlpNumLayers +#include "storage.hpp" // kMlpNumLayers, kDefaultMaxExamples namespace nisps::ml { @@ -40,7 +40,7 @@ class DynamicStorage { DynamicStorage(std::size_t n_in, std::span hidden, std::size_t n_out, - std::size_t max_examples = 128u, + std::size_t max_examples = kDefaultMaxExamples, std::size_t max_iter_train = 4096u) noexcept { if (hidden.size() != 3u || n_in == 0u || n_out == 0u || hidden[0] == 0u || hidden[1] == 0u || hidden[2] == 0u) { diff --git a/nisps/ml/mlp.hpp b/nisps/ml/mlp.hpp index eaba105..030e8a2 100644 --- a/nisps/ml/mlp.hpp +++ b/nisps/ml/mlp.hpp @@ -568,7 +568,7 @@ template using MLP = MLPCore< FixedStorage>; diff --git a/nisps/ml/storage.hpp b/nisps/ml/storage.hpp index 2828254..175d96a 100644 --- a/nisps/ml/storage.hpp +++ b/nisps/ml/storage.hpp @@ -47,12 +47,21 @@ namespace nisps::ml { inline constexpr std::size_t kMlpNumLayers = 4u; +// Default example-store capacity, named ONCE and shared by FixedStorage's +// compile-time default (below) and DynamicStorage's runtime-default +// constructor argument (nisps/ml/dynamic_storage.hpp). `nisps_ml_describe` +// (nisps/wasm/bindings.cpp) reports the live instance's max_examples() so +// the Manifold TS side (manifold/src/engine/wasm-iml.ts) can size its JS +// Dataset mirror to match instead of hardcoding a second, divergent number +// (see docs/specs/recon/simplification-audit-2026-07.md S35). +inline constexpr std::size_t kDefaultMaxExamples = 128u; + template class FixedStorage { public: diff --git a/nisps/wasm/bindings.cpp b/nisps/wasm/bindings.cpp index 215435c..814837d 100644 --- a/nisps/wasm/bindings.cpp +++ b/nisps/wasm/bindings.cpp @@ -868,10 +868,14 @@ void nisps_ml_clear_examples(void* ml) { h->mlp.clear_examples(); } -// Architecture introspection — writes [in, h1, h2, h3, out, n_layers] into a -// caller-supplied int buffer. Always 6 ints. With a null handle it reports -// the DEFAULT shape (what create() yields for non-positive args); with a -// handle it reports that instance's actual runtime shape. +// Architecture introspection — writes [in, h1, h2, h3, out, n_layers, +// max_examples] into a caller-supplied int buffer. Always 7 ints. With a +// null handle it reports the DEFAULT shape (what create() yields for +// non-positive args); with a handle it reports that instance's actual +// runtime shape. max_examples is the example-store (dataset) ring-buffer +// capacity (nisps::ml::kDefaultMaxExamples, nisps/ml/storage.hpp) — the +// single source of truth the TS Dataset mirror sizes itself to instead of +// hardcoding a second, potentially-divergent number (S35). EMSCRIPTEN_KEEPALIVE void nisps_ml_describe(void* ml, int* out_dims) { if (!out_dims) return; @@ -882,6 +886,7 @@ void nisps_ml_describe(void* ml, int* out_dims) { out_dims[3] = static_cast(kDefaultHidden[2]); out_dims[4] = static_cast(kDefaultOutputs); out_dims[5] = static_cast(BrowserMLP::kNumLayers); + out_dims[6] = static_cast(nisps::ml::kDefaultMaxExamples); return; } auto* h = static_cast(ml); @@ -891,6 +896,7 @@ void nisps_ml_describe(void* ml, int* out_dims) { out_dims[3] = static_cast(h->mlp.fan_out(2u)); out_dims[4] = static_cast(h->mlp.n_out()); out_dims[5] = static_cast(BrowserMLP::kNumLayers); + out_dims[6] = static_cast(h->mlp.max_examples()); } // --------------------------------------------------------------------------- diff --git a/tests/cpp/parity_wasm.mjs b/tests/cpp/parity_wasm.mjs index d359e98..29cdc19 100644 --- a/tests/cpp/parity_wasm.mjs +++ b/tests/cpp/parity_wasm.mjs @@ -7,19 +7,12 @@ * The WASM module is loaded from manifold/public/nisps.{js,wasm} — * scripts/build-wasm.sh must have run first. * - * Output blob format matches parity_check.cpp: - * uint32 magic = 'NPRT' - * uint32 version = 1 - * uint32 n_floats - * float32[n_floats] payload - * - * Payload order: - * 126 outputs (stage 1: post-process at (0.25, 0.75)) - * 12 weights (probed at fixed indices) - * 126 outputs (stage 2: post-train, re-process) - * 1 final training loss - * 2 PAFSynth L+R means (silence input, 128 samples) - * 2 ChannelStrip L+R means (0.25 input, 128 samples) + * Blob format and payload order are defined by parity_check.cpp — its header + * and `---- Stage N ----` sections are the authoritative stage list (seven + * stages: ML inference, ML training, PAFSynth, ChannelStrip, feedback + * controller, geometric dislike, pipelines + curves). Format: magic 'NPRT', + * version 5, n_floats, float32 payload. Keep the two drivers in lockstep and + * bump VERSION in both (and in parity_diff.mjs) on any layout change. * * Exit codes: * 0 success @@ -197,13 +190,17 @@ async function main() { const api = bind(Module); // Verify dimensions match the native side. A null handle reports the - // DEFAULT shape (what create() yields for non-positive args). - const dimsBuf = api.malloc(6 * 4); + // DEFAULT shape (what create() yields for non-positive args). 7 ints: + // [in, h1, h2, h3, out, n_layers, max_examples] (S35 — the buffer size + // and view length below MUST track nisps_ml_describe's actual output or + // this silently overflows the WASM heap by 4 bytes). + const dimsBuf = api.malloc(7 * 4); api.describe(0, dimsBuf); - const dims = new Int32Array(Module.HEAP32.buffer, dimsBuf, 6).slice(); + const dims = new Int32Array(Module.HEAP32.buffer, dimsBuf, 7).slice(); api.free(dimsBuf); - // Expect: [32, 10, 14, 18, 126, 4] (32-input max for mix-and-match) - const expectedDims = [32, 10, 14, 18, 126, 4]; + // Expect: [32, 10, 14, 18, 126, 4, 128] (32-input max for mix-and-match; + // 128 = nisps::ml::kDefaultMaxExamples, nisps/ml/storage.hpp) + const expectedDims = [32, 10, 14, 18, 126, 4, 128]; for (let i = 0; i < expectedDims.length; ++i) { if (dims[i] !== expectedDims[i]) { console.error(`[parity_wasm] WASM build has dim[${i}]=${dims[i]}, native expected ${expectedDims[i]}`); diff --git a/tests/cpp/test_mlp_storage_defaults.cpp b/tests/cpp/test_mlp_storage_defaults.cpp new file mode 100644 index 0000000..855a83b --- /dev/null +++ b/tests/cpp/test_mlp_storage_defaults.cpp @@ -0,0 +1,111 @@ +// tests/cpp/test_mlp_storage_defaults.cpp — regression test for S35 (dual +// example-store cap mismatch), docs/specs/recon/simplification-audit-2026-07.md. +// +// THE BUG (confirmed by trace, not just the audit's description): Manifold's +// WasmIML kept a JS `Dataset` mirror hardcoded to `maxSize = 100` +// (manifold/src/engine/dataset.ts / wasm-iml.ts) while pushing every example +// into the SAME C++ FIFO ring (`MLPCore`, the browser MLP — +// nisps/wasm/bindings.cpp `BrowserMLP`) whose cap defaulted to 128 +// (nisps/ml/dynamic_storage.hpp). Past 100 examples the two stores held +// DIFFERENT data: `train()` reads the C++ ring (up to 128 examples) while +// `trainAsync()` reads the JS Dataset (capped at 100) — silently diverging. +// The same mismatch produced a latent OOB read: `nisps_ml_train` +// (bindings.cpp) builds `std::span(sample_weights, +// mlp.example_count())` — sized to the C++ side's count (up to 128) — over a +// buffer the JS side allocated at its own (<=100) example count. +// +// THE FIX: name the shared capacity ONCE — `nisps::ml::kDefaultMaxExamples` +// (nisps/ml/storage.hpp) — used as FixedStorage's default template argument +// AND DynamicStorage's default constructor argument (nisps/ml/dynamic_storage.hpp), +// and exposed through `nisps_ml_describe`'s extended (7-int) dims descriptor +// so the TS side reads it instead of hardcoding a second, divergent literal. +// +// This test lives entirely on the C++ side (the WASM bindings link against +// emscripten headers and aren't part of the native ctest build), so it +// cannot exercise the TS Dataset mirror directly. What it CAN and DOES pin: +// 1. FixedStorage's and DynamicStorage's DEFAULT capacities are the same +// named constant, not two independently-hardcoded literals that could +// silently drift apart (the exact bug shape, reproduced within reach +// of a native test). +// 2. The actual runtime ring — constructed exactly as +// nisps/wasm/bindings.cpp's MLHandle constructs the browser MLP (no +// max_examples argument passed) — saturates AT that constant, not at +// the old wrong TS-side literal (100) and not unbounded. + +#include +#include + +#include "../../nisps/ml/dynamic_storage.hpp" +#include "../../nisps/ml/mlp.hpp" +#include "test_helpers.hpp" + +namespace { + +constexpr std::size_t kIn = 3u; +constexpr std::size_t kH1 = 4u; +constexpr std::size_t kH2 = 5u; +constexpr std::size_t kH3 = 6u; +constexpr std::size_t kOut = 2u; +constexpr std::uint64_t kSeed = 0xC0FFEEu; + +} // namespace + +// The two storage policies' DEFAULT example-store capacity must be the SAME +// named constant. If a future edit hardcodes a new literal in one place +// without updating the other, this fails immediately — no examples need to +// be pushed to catch it. +NISPS_TEST(storage_defaults_share_one_named_capacity) { + // FixedStorage's own default template argument (storage.hpp). + using DefaultFixedStorage = + nisps::ml::FixedStorage; // NMaxExamples defaulted + NISPS_EXPECT(DefaultFixedStorage::kMaxExamples == nisps::ml::kDefaultMaxExamples); + + // DynamicStorage's own default constructor argument — this is the exact + // path nisps/wasm/bindings.cpp's MLHandle uses for the browser MLP (no + // max_examples argument passed to the MLPCore ctor). + const std::size_t hidden[3] = {kH1, kH2, kH3}; + nisps::ml::MLPCore dyn( + kSeed, kIn, std::span(hidden), kOut); + NISPS_ASSERT(dyn.valid()); + NISPS_EXPECT(dyn.max_examples() == nisps::ml::kDefaultMaxExamples); + + // Cross-policy: the two independently-defaulted storage models must + // agree — this IS the invariant S35 violated across the C++/TS boundary. + NISPS_EXPECT(DefaultFixedStorage::kMaxExamples == dyn.max_examples()); + + // Pin the actual value: this is what nisps_ml_describe reports (out_dims[6]) + // and what every WASM caller must size its JS Dataset mirror to — 128, NOT + // the 100 the TS side used to hardcode. + NISPS_EXPECT(nisps::ml::kDefaultMaxExamples == 128u); +} + +// Push more than the OLD (wrong) TS-side cap of 100 examples through a +// default-constructed DynamicStorage MLP (mirroring MLHandle's construction +// exactly — no max_examples argument). The ring must saturate at +// kDefaultMaxExamples (128), matching what describe() reports — not at 100, +// and not unbounded. Before the S35 fix, Manifold's JS Dataset mirror capped +// at a hardcoded 100 while this ring kept growing past it: past 100 +// examples the two stores held different data (train() vs trainAsync()), +// and a sample-weight buffer sized to the JS side's (<=100) count would read +// out of bounds against this ring's (up to 128) example_count(). +NISPS_TEST(storage_defaults_ring_caps_at_shared_constant_past_old_ts_cap) { + const std::size_t hidden[3] = {kH1, kH2, kH3}; + nisps::ml::MLPCore dyn( + kSeed, kIn, std::span(hidden), kOut); + NISPS_ASSERT(dyn.valid()); + + constexpr std::size_t kOldWrongTsCap = 100u; + const std::size_t push_count = nisps::ml::kDefaultMaxExamples + 5u; // 133 > 128 > 100 + NISPS_ASSERT(push_count > kOldWrongTsCap); + + float feat[kIn]; + float lab[kOut]; + for (std::size_t e = 0; e < push_count; ++e) { + for (std::size_t i = 0; i < kIn; ++i) feat[i] = 0.01f * static_cast((e + i) % 17u); + for (std::size_t i = 0; i < kOut; ++i) lab[i] = 0.02f * static_cast((e + i) % 11u); + dyn.add_example(std::span(feat, kIn), std::span(lab, kOut)); + } + + NISPS_EXPECT(dyn.example_count() == nisps::ml::kDefaultMaxExamples); + NISPS_EXPECT(dyn.example_count() != kOldWrongTsCap); +}