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 (L34, L35). Both findings' line citations were accurate this time.
- L34 data race: process() (Rack's audio thread) called
imlShadow.get_example_features()/get_example_labels() directly on the WORKER
thread's private engine — which the file's own THREADING INVARIANT comment
says only the worker may touch — while workerLoop() concurrently
clear/refills those same std::vector<std::vector<float>> members via
load_examples(), train_(), randomise_weights and clear_dataset. Unsynchronised
reader/writer on a non-atomic vector: undefined behaviour.
Fix extends the staged handoff the file ALREADY uses for pendingWeights
rather than adding a second mutex: the worker deep-copies features/labels
into pendingFeatures/pendingLabels at the same instant it copies
pendingWeights, immediately before weightsPending.store(true), and the flag is
now released only after the whole batch is consumed — closing an early-release
window the old code had. process() no longer references imlShadow at all
(verified: the only surviving mention is a comment).
- L35: process() ran full jansson serialize on every weight-swap OSC push and
full json_loads + dataFromJson on incoming OSC state — heap-heavy tree work
on the audio thread. The plan said to move it to the worker. It is DELETED
instead: reading the actual transport shows both directions talk to nobody —
osc-client.ts only ever sends {params|input|feedback}, and bridge.ts has no
state/weights case and explicitly drops other addresses. Relocating
heap-heavy work to serve a confirmed-zero consumer is complexity without a
requirement; removing the cause is the smaller coherent design.
dataToJson/dataFromJson are UNTOUCHED — they remain the live consumers for
Rack patch save/load and the .nisps preset menu, both off the audio thread.
Neither is empirically reproduced: a real race needs a live Rack engine under
TSan, which is not available here. Justified by reading, and verified by
`cd vcv && make -j4` (clean) plus the host suite including
test_vcv_iml_parity.cpp, which pins iml.hpp bit-exactly against the core MLP —
iml.hpp was not modified, and parity holds.
Known remaining, pre-existing and out of scope: process() still takes a brief
lock_guard on feedbackMutex to copy a small staged struct, and several config
fields (slewMs, oscPort, output/input range flags) are written by the UI thread
without atomics.
Phase 1 group 7 (L33). vcv/test/smoke_test.cpp included a header that no longer
exists (a retired nisps-core path), asserted the pre-P6 2x12 module shape, and
ran in no gate; a compiled smoke_test binary was tracked alongside it. Removed
the directory plus Makefile.dist, updated BUILDING.md's two references, and
deleted the unreachable reply-to-sender branch in osc_server.hpp.
Gates: run-all-tests.sh ALL GREEN.
Replace the vendored runtime MLP in vcv/src/iml.hpp (DetRng + 3D-weight-store
MLP + Dataset + IML) with a THIN, Rack-free adapter over the shared core:
nisps::ml::MLPCore<nisps::ml::DynamicStorage> (8->[16,24,16]->16, the P2 dynamic
case), nisps::Rng, and the core MLP's own FIFO dataset. Behaviour changes from
the vendored approximation to core-exact firmware/WASM semantics.
- MEMLNaut.cpp: staged/pending weight buffers and patch JSON now use the core's
flat [weights..][biases..] vector (nisps::IML<float>::Weights); patch version
bumped to 3. Double-buffer / single-writer threading discipline unchanged.
- New ctest tests/cpp/test_vcv_iml_parity.cpp: seeded train/infer/move_weights
session through the adapter is memcmp-equal to a bare MLPCore<DynamicStorage>.
- Docs: vcv-module.md delta #5 marked CLOSED (2026-07-18); MAP.md vcv/ updated.
Closes vcv-module.md delta #5.
8 inputs x 16 outputs; per-output LED ring widget (drawLayer+nvgArc); palette
from frontend tokens; OSC bridge verbs for bidirectional browser training;
vendored self-contained iml.hpp (retired nisps-core); compiles against Rack
SDK 2.6.4. See SPEC.md BUILD DELTAS.
Fixes from Opus 4.6 review (C1-C6, I1, I3, I4, I8):
C1: static lastOutputs → per-instance lastOutputsForDelta member
C2: add_example() now on audio thread only (owns iml); worker reads
from mutex-protected staging area (stagedFeatures/stagedLabels)
C3: Worker reads stagedWeightsForWorker (not iml.get_weights()),
eliminating concurrent read/write on iml's MLP
C4: Worker spins on weightsPending before writing pendingWeights,
preventing double-write race
C5: RAND and CLEAR now enqueue Randomize/Clear jobs through worker
instead of directly mutating iml on the audio thread
C6: OSC callbacks stage JSON into oscStagedJson + atomic flag;
audio thread applies in process() (no recv-thread mutation)
Also fixed:
- I4: Separate pendingJob field (enqueueJob no longer overwrites currentJob)
- I8: Removed redundant swapReady atomic
- noiseLevel, cachedNovelty, cachedConfidence now std::atomic<float>
- Worker syncs examples back to iml after training via load_examples()
Replace direct-mutation threading with shadow IML:
- Background thread clones weights from main → shadow IML
- Training and perturbation operate only on shadow instance
- New weights staged in pendingWeights, swapped atomically by audio thread
- Thumbs-down now enqueues Perturb job instead of calling move_weights directly
- Audio thread applies new weights via iml.set_weights() at safe point
- Examples copied to shadow for training, results copied back as weights only
Threading invariant now fully enforced: background thread never writes to
the inference IML. Audio thread applies staged weights between inference calls.
