Upstream memlp (github.com/MusicallyEmbodiedML/memlp @ ea777502, the commit
upstream/main pins) applies gradients with RMSProp everywhere: Layer.h:239
ApplyAccumulatedGradients, the m_sq_grad_avg running average at Layer.h:601,
StaticMLP.h:268. nisps/ml/training.hpp shipped SGD only and filed the
difference as an optimiser-choice research question. It was not one.
RMSProp divides each step by the running gradient magnitude, so an upstream
lr is a NORMALISED step; under SGD the same number multiplies the raw
gradient. Every learning rate ported from upstream therefore landed in an
optimiser that reads it differently — most visibly feedback.hpp's
`geo_lr_ = 0.001f // upstream InterfaceRL.hpp:312`, an RMSProp LR pasted
into a single SGD step.
rmsprop_step() ports Layer.h:239 exactly: clip at +/-10, sq = min(0.9*sq +
0.1*g^2, 1e6), adj = min(lr/(sqrt(sq)+1e-6), 1.0), w -= adj*g. The
adjusted-LR clamp stays one-sided as upstream's std::min is, so the negative
lr used by train_targets' "train away from this target" path behaves as it
does upstream. The per-weight squared-gradient average is new persistent
state and lives in the storage policies (FixedStorage arrays /
DynamicStorage arena) so nisps/ stays allocation-free and the firmware's
zero-heap contract holds. It is optimiser state, not model state: excluded
from weight_count()/get_weights()/set_weights(), matching upstream, and
cleared by MLPCore::reset_optimizer_state() (upstream ResetOptimizerState).
draw_weights() deliberately does NOT clear it — upstream's DrawWeights
doesn't either.
Measured with tests/cpp/ml_bench.cpp:
D1 one geometric dislike moves the mapping 1.6e-2, up from 5.3e-5 (~295x),
and repeated presses now CONVERGE on the intended 0.5 push (0.12 at 10,
0.56 at 100) instead of creeping linearly forever.
A4 geometric-vs-Diffuse gap narrows from ~4100x to ~14x in one press.
U4 the upstream-LR positive path actually trains now (range_util 0.71 at
100 ticks/gesture, was 0.016 — it was inert under SGD).
Not fixed by this, and now tracked as ALIGNMENT defect 6d: the dose
asymmetry. lurch_max is still ~1.08 against a [0,1] output range.
Golden vector stages 2 and 3 re-captured; stages 0 and 1 are pre-training
and did not move, which is the cross-check that only the update rule
changed. manifold/public/nisps.wasm rebuilt so parity-check compares like
with like — it FAILED at up to 5e-2 against the stale artifact and PASSES at
2.4e-7 against a fresh one. parity-check.sh only builds the WASM when it is
missing, never when it is stale; noted in MAP.md and filed separately.
ALIGNMENT defect 6 resolved (moved to Recently resolved); 6b's optimiser
cross-reference updated; new defect 6d for the positive-training dose.
Gates: build-cpp-tests 138 tests / ctest 4/4, parity-check PASS, lint-cpp
clean, manifold typecheck clean.
The operator's call: "there should be one default learning rate and one
default max iterations and they should both be configurable at runtime."
There were SIX copies, not the four the audit described, and they did not
agree:
nisps/ml/mlp.hpp no-arg train() hardcoding 1.f / 1000u / 0.001f —
and firmware's ONLY training path calls exactly
this, so firmware had no runtime knob at all
wasm-iml.ts train() and trainAsync() TS default params (x2)
engine-api.ts learningRate ?? 1.0, with no maxIterations knob
vcv/src/iml.hpp 200 / 0.1 / 0.00001 — silently divergent
external_synth_midi.hpp its own kDefaultLearningRate/kDefaultMaxIterations
schemas/modes/*.json x9, identical, read by nobody at runtime
Now: schemas/ml_defaults.json is the single declaration (validated against a
sibling meta-schema, matching the midi_device.schema.json convention), codegen
emits it to C++ and TS in the same run, and MLPCore carries a TrainConfig whose
default member initialisers read the generated constant.
set_train_config()/nisps_ml_set_train_config() make it runtime-overridable on
every target; the explicit-argument train() overload is untouched. min_error
joins the tuple — it was duplicated identically and belongs with the other two.
The per-mode ml block loses default_learning_rate/default_max_iterations.
default_spread stays (genuinely wired on both targets) and input_channels stays
(codegen-time validated, real information for sound_analysis_midi).
VCV BEHAVIOUR CHANGE, deliberate: MEMLNaut.cpp constructs IML positionally and
relies on those defaults, so the module moves to 1000/1.0/0.001 — 5x the max
iterations, 10x the learning rate, and a 100x looser early-stop threshold. The
old values were never justified anywhere; they arrived with fbc68eb alongside
an unrelated module rewrite and no tuning rationale. Firmware and WASM have
shipped 1.0/1000 all along. It is now runtime-settable if this turns out worse.
The generated header lands in nisps/ml/generated/, not nisps/modes/generated/
where the rest of codegen output lives: training hyperparameters are an ML
fact, and nisps/ml sits below nisps/modes, so emitting them there would make
mlp.hpp include upward. The agent that built this flagged the directory-crossing
rather than hiding it; this is the fix. CI's generated-freshness gate learns the
new directory.
Gates: run-all-tests.sh ALL GREEN — 4/4 ctest, parity PASS (max delta 2.38e-7),
lint clean, manifold typecheck + 17 unit + 33 e2e (which exercise train() and
trainAsync() through a real browser).
Phase 2, S35. Two real defects from one root cause, both confirmed by trace
rather than taken from the audit:
1. Divergence. WasmIML built its TS Dataset mirror with a cap of 100 while
every addExample() ALSO pushed into the C++ FIFO ring, capped at 128. Since
train() reads the C++ ring and trainAsync() reads the TS mirror, past 100
examples the two trained on different datasets — silently.
2. Latent OOB read. nisps_ml_train sizes its sample-weight span by the C++
side's example_count() (up to 128), but wasm-iml.ts allocates that heap
buffer from the TS dataset's size (<=100). Once the ring exceeds the mirror,
the span reads past the end of the caller's allocation.
Fix: name the capacity ONCE as nisps::ml::kDefaultMaxExamples = 128, used by
FixedStorage's default template arg, DynamicStorage's default ctor arg, and the
MLP<> alias (which is the only real FixedStorage instantiation path and carried
its own independent 128 literal — the last copy of this dual truth). Expose it
through nisps_ml_describe and have the TS side read it instead of hardcoding.
Dataset's constructor default is removed entirely: a default was what invited
this bug class, and the sole call site now always supplies the describe() value.
ABI NOTE: this extends nisps_ml_describe from a 6-int to a 7-int descriptor.
nisps_ml_describe always writes 7 ints regardless of the caller's buffer, so
every call site had to grow in the same change or it would overflow the WASM
heap by 4 bytes per call. All five sites updated: three in wasm-iml.ts (init
defaults, init per-instance, reshape re-describe — the finding said there were
two), one in wasm-worker.ts, one in tests/cpp/parity_wasm.mjs. The parity
harness's expected-dims check now also pins the new max_examples slot.
Regression test: tests/cpp/test_mlp_storage_defaults.cpp — pins the two storage
policies to one constant, and drives MLPCore<DynamicStorage> exactly as
bindings.cpp does past the old TS cap, asserting it saturates at 128 and not at
100. Fail-before/pass-after confirmed by temporarily setting the constant to
100: 2 failures, named. Reverted: green.
Audit correction: the cited dataset.ts:81 is the FIFO eviction check; the
hardcoded default was at dataset.ts:45.
Gates: run-all-tests.sh ALL GREEN, parity PASS.
Algorithms (forward, backprop/SGD, init, move_weights, diagnostics) now live
once in MLPCore<Storage> (nisps/ml/mlp.hpp). Storage models:
- FixedStorage (storage.hpp): template-sized std::array, zero heap. The
classic MLP<NIn,H1,H2,H3,NOut,...> is an alias preserving kInput/kHidden*/
kOutput/kNumLayers/weight_count() constexpr — firmware + bindings + modes
compile unchanged.
- DynamicStorage (dynamic_storage.hpp): runtime dims, ONE arena allocation
at construction, nothing per-call. #error under NISPS_TARGET_EMBEDDED
(new macro in core/perf.hpp); sole lint-cpp.sh heap-allowlist entry, plus
a lint check that fails if the #error guard disappears.
Verification:
- new ctest test_mlp_storage_parity: fixed↔dynamic BIT-identical across
init/draw/inference/train(FIFO)/move_weights(pin mask)/eval_loss/
layer_stats/set_weights/infer_batch/reset; invalid+moved-from inert
- golden ML vectors (pre-refactor constants) pass → bit-stable refactor
- native↔WASM parity PASS, max delta unchanged (2.4e-7)
- chokepoint B compile: PAFSynth .text 122324→122692 (+0.30%, ±1% budget);
RAM +416B (eval scratch)
- fix: firmware-common.sh used bare 'python' (absent here) → ${PYTHON:-python3}
Part of one-core-engine-refactor P2. nisps_ml_create ABI untouched (P2.2 is
an operator stop-point).
monkey-w1n5t0n