From ec3118004daf0322a46304496da77517186ef354 Mon Sep 17 00:00:00 2001 From: monkey-w1n5t0n Date: Sat, 25 Jul 2026 11:22:15 +0200 Subject: [PATCH] =?UTF-8?q?fix(ml):=20re-base=20the=20geometric=20dislike?= =?UTF-8?q?=20on=20upstream=20e291192=20=E2=80=94=20delete=20the=20taper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit geo_push.hpp and replay.hpp cited memllib @ 0a541cc. upstream/main pins e291192, where the same code had been deliberately redesigned — and because InterfaceRL was not in the tree (fixed one commit ago), we carried the superseded version for months. Three changes, all upstream's: kGeometricPushScale 0.5 -> 1.0 (InterfaceRL.hpp:409) kNegLRBase 0.5 -> 1.5 (InterfaceRL.hpp:410) /(1+len) taper deleted (InterfaceRL.tpp:724) Upstream's own comment on the taper: "a 'no' should clearly move the mapping away even from a sound already far from the liked region (the taper used to kill exactly that case)". The direction is already a unit vector, so the taper only ever shrank the push for exactly the sounds a user is most likely to be rejecting. Cold start is folded into the same path. Upstream's useRandom is `!havePositives || len <= 1e-4`: with nothing liked yet there is no centroid to push away from, so every dim goes in a random direction. Ours instead kept the older 0a541cc fallback — train AWAY from the heard action at a NEGATIVE lr — which was inert whenever the heard action equalled the net's own output, i.e. in the common case. One path now, and a "no" moves the mapping before any likes exist (ml_bench E1: 0 -> 2.3e-3). The GeometricColdStart action is still reported so callers keep their "like a few sounds first" prompt; only the training changed. Measured (ml_bench, one dislike at a point): A4 0.0157 -> 0.0533 at-point displacement (3.4x), so end to end across this and the RMSProp fix: 5.3e-5 -> 5.3e-2, ~1000x. The gap to the legacy Diffuse design closes from ~4100x to ~4.2x. D1 effective_lr 4.7e-4 -> 1.5e-3; 10 presses now reach 0.34, 100 reach the full intended push. A5 compounding 0.87 -> 0.96 (a second press at the same spot is no longer noticeably weaker than the first). A7 damage_ratio essentially unchanged (0.87-1.57) — the collateral damage to protected positives scales with the push and is NOT addressed here; it is the negative-feedback design question. NOT adopted, deliberately: upstream's per-tick batch retraining over all live negatives, and its fixed kDislikeLifetimeMs=2500 in place of our proportional decay. Both need something the core does not have — a per-tick call site and a millisecond clock inside nisps/ml — so they change FeedbackControllerCore's interface rather than its constants. Recorded in ALIGNMENT's deferred-debt entry alongside the existing one-press-one-step divergence, and filed as its own task. test_mlp_geo_dislike.cpp: the taper test now pins its ABSENCE (equal displacement near and far), the cold-start test pins movement where it used to pin inertness, and a new test covers the random-direction branch. ALIGNMENT defect 6b resolved. Gates: build-cpp-tests 139 tests / ctest 4/4, parity-check PASS (WASM rebuilt), lint-cpp clean, firmware slpworkshop SUCCESS. --- ALIGNMENT.md | 35 +++++-------- MAP.md | 2 +- manifold/public/nisps.wasm | Bin 129293 -> 129136 bytes nisps/ml/feedback.hpp | 58 ++++++++++++--------- nisps/ml/geo_push.hpp | 56 ++++++++++++++------- tests/cpp/test_mlp_geo_dislike.cpp | 78 +++++++++++++++++++---------- 6 files changed, 135 insertions(+), 94 deletions(-) diff --git a/ALIGNMENT.md b/ALIGNMENT.md index f6d05e9..82a82d2 100644 --- a/ALIGNMENT.md +++ b/ALIGNMENT.md @@ -63,28 +63,6 @@ command surface to report through. **Rough cost.** Host half is done. On-device: ~a day, and it wants defect 3's serial protocol to have somewhere to send the number. -### 6b. The geometric dislike was ported from a superseded upstream design (2026-07-25) - -**What.** `geo_push.hpp`/`replay.hpp` cite `memllib @ 0a541cc`. `upstream/main` now pins -`e291192`, where the same code has been deliberately redesigned. Upstream: -`kGeometricPushScale` 1.0 (ours 0.5); neg-LR base 1.5 (ours 0.5, `geo_push.hpp:92`); -the `/(1+len)` taper **deleted**, with the comment "a 'no' should clearly move the -mapping away even from a sound already far from the liked region (the taper used to -kill exactly that case)" — ours still applies it at `geo_push.hpp:66`; negatives trained -as a **batch over ALL of them every tick** rather than one item one step; and a fixed -`kDislikeLifetimeMs = 2500` full-strength lifetime replacing the proportional decay we -ported. On the shared constants (dedup radius 0.05, `kCentroidK` 4) we match. - -**Why it blocks the mission.** We are carrying a design upstream diagnosed and fixed, -and the fix is documented in their source comments. On the constants alone the ported -dislike is ~9.4x weaker than upstream's. (The optimiser half of this — an RMSProp LR -pasted into an SGD step — is fixed as of 2026-07-25; see "Recently resolved". A single -dislike now moves the mapping 1.6e-2 instead of 5.3e-5, but that is still ~14x weaker -than the legacy Diffuse design measures in one press, `ml_bench` A4.) - -**Rough cost.** Small — mostly deleting the taper and re-basing three constants, then -re-running `scripts/bench-ml.sh` D1/A4/A7 to confirm. - ### 6d. One like still heaves the whole mapping (2026-07-25) **What.** A thumbs-up trains at `lr 1.0 x 1000 iterations` on every gesture. `ml_bench` @@ -124,11 +102,22 @@ Legacy a-immersive was mobile-first; Manifold is desktop-first. Defer until user - **EOC effects chain, ShapeSeq sequencer, modular engine (Phase E)** — legacy features consciously out of the v1 rewrite; revisit only if a mode wants them. - **Inputs multi-source composition** (2026-06-28, reaffirmed 2026-07-21) — mix-and-match pad+gamepad+MIDI is a recorded, unreversed decision; the UI currently enforces exclusive single-source and the composition machinery sits dormant *by design*. Schedule or keep dormant — but the inputs-spec must stop presenting composition as current behaviour (plan §8). - **Schema content is partially placeholder** (2026-07-21) — 20 anonymous "Param NN" slots across paf_synth/channel_strip/xiasri and copy-pasted ML defaults across all 9 modes. Name them during the first curated-preset pass per mode (plan §6.5c), or shrink `output_size` where the engine allows. -- **Geometric-dislike deliberate divergences** (2026-07-14, one-core P3): (1) the degenerate-branch RNG draws from the controller's deterministic `nisps::Rng`, not libc `rand()` — native==WASM parity holds; (2) upstream's async shuffled two-LR `optimise()` is collapsed into one synchronous `dislike_geometric()` training only the pressed negative's target — behavioural, not bitwise, parity with firmware upstream, by design; (3) `RandomiseMlp` uses `draw_weights(spread)` rather than the old asymmetric ranges. All intentional. +- **Geometric-dislike deliberate divergences** (2026-07-14, one-core P3; reaffirmed 2026-07-25): (1) the degenerate-branch RNG draws from the controller's deterministic `nisps::Rng`, not libc `rand()` — native==WASM parity holds; (2) upstream's async shuffled two-LR `optimise()` is collapsed into one synchronous `dislike_geometric()` training only the pressed negative's target — behavioural, not bitwise, parity with firmware upstream, by design; (3) `RandomiseMlp` uses `draw_weights(spread)` rather than the old asymmetric ranges. All intentional. The 2026-07-25 re-base onto `e291192` took upstream's constants and deleted the taper but did NOT adopt (2)'s counterpart — upstream retrains a batch over ALL live negatives on every 200 Hz tick and holds each negative at full strength for a fixed `kDislikeLifetimeMs = 2500` instead of decaying it proportionally. Adopting that needs a per-tick call site and a millisecond clock inside `nisps/ml`, which the core does not have; it is a real interface decision, not a constant, and is tracked as its own task. - **Manifold dock splits `state`/`muted`/`armed`** (2026-06-28) — deliberate divergence from the deployed conflated `frozen`↔`muted` model (dock-spec §3.3). `muted`-downstream and the `soloMode` gradient-mask variants remain UI-only; the C API exposes `set_focus` but no per-mode gradient masking yet. (The audit found `soloMode` behaviourally inert in the controller — plan L20 trims it until `train_masked` exists.) ## Recently resolved (delete after a few weeks) +- 2026-07-25: **The geometric dislike is re-based on upstream `e291192` (defect 6b).** + `kGeometricPushScale` 0.5 -> 1.0, `kNegLRBase` 0.5 -> 1.5, and the `/(1+len)` taper + deleted — upstream's own comment is that a "no" should clearly move the mapping away + even from a sound already far from the liked region, which is exactly the case the + taper killed. Cold start folded into the same path (random direction when nothing is + liked yet) instead of the superseded negative-LR branch, so a "no" now moves the + mapping before any likes exist (`ml_bench` E1: 0 -> 2.3e-3). One dislike moves the + mapping **5.3e-2**, up from 1.6e-2 after the RMSProp fix and 5.3e-5 before it — a + ~1000x change end to end, and now within ~4x of the legacy Diffuse design instead of + ~4100x (`ml_bench` A4). Not adopted: upstream's per-tick batch retraining and fixed + 2500 ms dislike lifetime — see "Deferred / accepted debt". - 2026-07-25: **`InterfaceRL` is back in the tree (defect 6c).** Vendored verbatim from memllib `e291192` at `firmware/MEMLNaut-NISPS/lib/memllib/reference/` — outside `src/`, so PlatformIO never compiles it. Upstream drift in the feedback subsystem is a `diff` diff --git a/MAP.md b/MAP.md index 26f11a9..286386e 100644 --- a/MAP.md +++ b/MAP.md @@ -6,7 +6,7 @@ MEMLNaut-NISPS — Neural Interactive Shaping of Parameter Spaces. One C++20 cod ### `nisps/` — platform-agnostic C++20 library (the only ML/DSP/engine code) - `nisps/core/` — `perf.hpp` (hot-path/inlining attrs), `types.hpp`, `concepts.hpp` (`MLEngine`, `AudioEngine`, `Mode`), `ring_buffer.hpp` (SPSC lock-free cross-core channel, replaces pico/util/queue), `event_queue.hpp` (single-threaded in-engine event FIFO — deliberately NOT RingBuffer, which is an atomics-based cross-thread channel), `rng.hpp` (xoshiro256+ deterministic), `math.hpp` (fast_sigmoid, `Curve` enum + `apply_curve`). -- `nisps/ml/` — the MLP core, written once against a storage policy (`mlp.hpp` `MLPCore`): `storage.hpp` (`FixedStorage` — template-sized `std::array`, zero heap; `MLP` alias preserves the classic compile-time surface) and `dynamic_storage.hpp` (`DynamicStorage` — runtime dims, single arena alloc at construction; `#error`s on RP2350 builds, sole lint heap-allowlist entry). Fixed↔dynamic bit-parity enforced by `tests/cpp/test_mlp_storage_parity.cpp`. Files: `mlp.hpp`, `activations.hpp`, `loss.hpp` (MSE, no double-scaling), `training.hpp` (**RMSProp** + grad clipping — `rmsprop_step()` is a line-for-line port of upstream memlp `Layer.h:239 ApplyAccumulatedGradients` @ `ea777502`; the per-weight running squared-gradient average is optimiser state held in the storage policies, NOT part of `weight_count()`/`get_weights()`, and `MLPCore::reset_optimizer_state()` clears it. `draw_weights()` deliberately does not, matching upstream `DrawWeights`), `init.hpp` (spread-aware uniform↔Xavier), `rl.hpp` (`move_weights` with output pin mask + per-layer scaling + weight decay), `jolt.hpp` (`Jolt` — held continuous weight-morph over the flat weight buffer + post-release LR ramp; ported from upstream InterfaceRL), `ou_noise.hpp` (`OUNoise` — Ornstein-Uhlenbeck exploration walk on the output vector; ported from upstream InterfaceRL), `feedback.hpp` (`FeedbackControllerCore` — the "Down Action" state machine: Avoid (geometric push-away default / Diffuse legacy) / RandomiseOutputs / RandomiseMlp / ExploreAndPlace; storage-policied like the MLP, own deterministic RNG, exposed via `nisps_ml_feedback_*` C API), `replay.hpp` (`ReplayView` — reward-tagged memory: dedup/deepen, k-NN positive centroid with deterministic tie-break, proportional decay+eviction), `geo_push.hpp` (push-away target computation, upstream InterfaceRL @ 0a541cc), `warm_start.hpp` (overlapping-weights copy for reshape), `stats.hpp`. `generated/ml_defaults.hpp` is codegen output (do not edit): `nisps::ml::generated::kMlTrainDefaults`, the ONE learning-rate / max-iterations / min-error default shared by firmware, WASM and VCV (source `schemas/ml_defaults.json`); `MLPCore::set_train_config()` and `nisps_ml_set_train_config()` override it at runtime. It lives under `ml/` rather than `modes/generated/` because `nisps/ml` sits below `nisps/modes` — mlp.hpp must not include upward. Jolt + OU are inert by default and wired into `ModeBase`, so every mode exposes `jolt_press/jolt_release`, `jolt_lr_scale`, and `set_explore_intensity`. +- `nisps/ml/` — the MLP core, written once against a storage policy (`mlp.hpp` `MLPCore`): `storage.hpp` (`FixedStorage` — template-sized `std::array`, zero heap; `MLP` alias preserves the classic compile-time surface) and `dynamic_storage.hpp` (`DynamicStorage` — runtime dims, single arena alloc at construction; `#error`s on RP2350 builds, sole lint heap-allowlist entry). Fixed↔dynamic bit-parity enforced by `tests/cpp/test_mlp_storage_parity.cpp`. Files: `mlp.hpp`, `activations.hpp`, `loss.hpp` (MSE, no double-scaling), `training.hpp` (**RMSProp** + grad clipping — `rmsprop_step()` is a line-for-line port of upstream memlp `Layer.h:239 ApplyAccumulatedGradients` @ `ea777502`; the per-weight running squared-gradient average is optimiser state held in the storage policies, NOT part of `weight_count()`/`get_weights()`, and `MLPCore::reset_optimizer_state()` clears it. `draw_weights()` deliberately does not, matching upstream `DrawWeights`), `init.hpp` (spread-aware uniform↔Xavier), `rl.hpp` (`move_weights` with output pin mask + per-layer scaling + weight decay), `jolt.hpp` (`Jolt` — held continuous weight-morph over the flat weight buffer + post-release LR ramp; ported from upstream InterfaceRL), `ou_noise.hpp` (`OUNoise` — Ornstein-Uhlenbeck exploration walk on the output vector; ported from upstream InterfaceRL), `feedback.hpp` (`FeedbackControllerCore` — the "Down Action" state machine: Avoid (geometric push-away default / Diffuse legacy) / RandomiseOutputs / RandomiseMlp / ExploreAndPlace; storage-policied like the MLP, own deterministic RNG, exposed via `nisps_ml_feedback_*` C API), `replay.hpp` (`ReplayView` — reward-tagged memory: dedup/deepen, k-NN positive centroid with deterministic tie-break, proportional decay+eviction), `geo_push.hpp` (push-away target computation, upstream InterfaceRL @ `e291192` — re-based from `0a541cc` on 2026-07-25: `kGeometricPushScale` 1.0, `kNegLRBase` 1.5, and NO `/(1+len)` taper, so distance from the liked centroid does not shrink a "no". Cold start is the same push in a random direction, not a separate branch), `warm_start.hpp` (overlapping-weights copy for reshape), `stats.hpp`. `generated/ml_defaults.hpp` is codegen output (do not edit): `nisps::ml::generated::kMlTrainDefaults`, the ONE learning-rate / max-iterations / min-error default shared by firmware, WASM and VCV (source `schemas/ml_defaults.json`); `MLPCore::set_train_config()` and `nisps_ml_set_train_config()` override it at runtime. It lives under `ml/` rather than `modes/generated/` because `nisps/ml` sits below `nisps/modes` — mlp.hpp must not include upward. Jolt + OU are inert by default and wired into `ModeBase`, so every mode exposes `jolt_press/jolt_release`, `jolt_lr_scale`, and `set_explore_intensity`. - `nisps/pipeline/` — the control-rate input/output processing chains (P4): `input_chain.hpp` (`InputChain` — invert→deadzone→circular clamp→momentum-modulated zoom→centred power→EMA→momentum; caller-supplied dt, internal clock, fixed velocity ring, serialisable state) and `output_chain.hpp` (`OutputChain` — curve→EMA→slew→freeze(+mask), capacity-templated). Behaviour contract = the retired manifold TS pipelines, pinned by `manifold/tests/fixtures/` and parity stage 7. - `nisps/dsp/` — `biquad.hpp`, `delay.hpp`, `reverb.hpp`, `filter.hpp`, `env.hpp`, `osc.hpp`, `pitch_shift.hpp`, `dc_blocker.hpp`, plus the sequencer primitives shared by the sequencer engines: `ratio_seq.hpp` and `seq_clock.hpp` (bar phasor + MIDI clock + bpm). Lean primitives extracted from maximilian; daisysp PitchShifter replaced with custom granular impl. - `nisps/engines/` — eight audio engines, each satisfying `AudioEngine`: `paf_synth.hpp`, `channel_strip.hpp`, `xiasri.hpp`, `verb_fx.hpp`, `memlcelium.hpp`, `breakor.hpp` (sequencer, NoOp audio), `elysiamorf.hpp` (sequencer, NoOp audio), `analysis.hpp` (input-side spectral features). Plus `base.hpp` (`NoOpEngine`, engine_id "thru"). diff --git a/manifold/public/nisps.wasm b/manifold/public/nisps.wasm index 593193b14e8f704c8c9e033612178ac5c19e8777..7c290a00aa686c16c6ddd10a7e38370505d4790e 100755 GIT binary patch delta 7487 zcmeHMU2I&%6`q-U@812nyIIGM3XX(aCzR?7c_%zVt*} zR%Uaa3gjZuzSt8TOO}e9Cptx!Cn71Nmrp5Pt(3ejVwJ>oujueev=t*ZYE=xWN2_A1daNq8r~_58Sv_7Ao759ku|XZI ziuG!^Dk|#9su)zYs#vE+s^WGvS`}+m=1^6vQHQJIw))4aBloPDsETK@Z(j%}XaAve zTK3f!MqvpQL90aOnm=KD*lMih~!z7nh657jnd;y9J4oD(MW9RXWRLjSiUHs8dWL9WoKR z$fT%!N|UHu5Vf-UOrq_V)hA4gWp#;ZSXLh=;}(bhuDnIP=eu; z)XaP}Nbs`!unm|(2U8yqD{1QeX+uPIU{B|2oG@Kd_c`@<83l&HHmHA7URwR9c?YLV zn6#vJ>JYLhmIuo8cfd@k%CRvWVI&K^LNL~ArbZ!U`1rhoPZ`#8o(J>IgXymujK&+w z<79nM@mb$l5R;|I={r^N#!LosUfwm!8)O?BSVaZ!R;X7|#wr+|55L^0d)eEFvfJ6s zDx%oUednDzo1;9dy;f%XW4sIM21)Az)Q?7o2@?kvyr0uf%K0%o74Dk1`U$PGQOKU# z!dmkN6JJCSSvBQquwyi2J;y4>IctuU7Ew@N`+0YqI2(eFkI~`|57uKWL&8VR@lj5} zm-*x%v(HltFhe41NWC0$2>)J$mX#Gwh`@j^LROBNIay=E8LhA|A8gLTMptfP<&ZGu zHt@h#c1YMxBOqXL?h)~s=AR|!&VsG&S(0t+S?o570|+)~It;772;+cbWol~b-#;z` zzEtc`rUZm#cnKK%0KRMZ&n-OH29N^o;bbAZ@dc@2b~_9`J3kKCkFyx{;ZYG~;MiDN z|L5m7#SA}i!4&iZ7la<2>&vUoB#AIG*v-dnSvg;-6&1K@%oDZ7d{NVZ0-HEqpK z6Jl%ls@vLR6K1j4S0i+M%h|QlP3eP# zCC41z#GC!WMC6c@N5EbZG)6#KV|q^PJa`+L#t1pVOs*4{gJIw3X(H0eaK2M(I0&L3 zUxQtsZ>M8;I&j2F0ZVF%6NOyG*JlPkg;<@#ym0ok0;#d~cx1nQm}Iw}Bg@ zWFA$+V9n&Z1=I+qbKbd#gApLK)%4iJXJPeYiOh$zh0NE=%5A4sPIL=-**O{O#fj5i z&*)OHdEolFFmu`j*Tn#?B=fO%ySTBL&*un%6s-kRktjHUV;qbV2ADc-mU6HHAS|sL zLuLlwEs!EV6>5yb$?&`ci2TczYIpv!(1N04>syXK);2F?JfKrfAPRM0&*7tr3K+JKlev!=JGzXeVm!)GM-6|L)nIf+5{B~;2tfgIE0mOTT;hOdAy!9BN6mD)$pAwZ z(;(?&!Nvp$&rY{0*{UO9joCaanS*??%@z%OB8cfD0zd0{aI6-qmTsYAp$Y&whE^^6>vlGX2 zf!Yxtm;{qJxy(YiW!z%Txe(G%v)Z_3v2~MKOf9Z#qQ%W0k=Ga2uJsZ59pJ=#uV9;` z`2u5Cf!eHBK`89s+ALIo7Z&L#8Q%R4|rluWH-h;aVLCuXH^1!FbF z@QcVwj?U(&(Uocby0wn@U_|XpfMb^GP!y1jsJj0usL zYPZlC#5`Pi4U#*%^bDD424<@ksLC~`pehc?#Os6lp}#DN zQ9A+saK;;i;FJebve9s2D$MuRiC9%*XsCU4d1St)8M?iW8y!{|62E1V$ zh!c}yC0<*Xn(V=`VX~j;cMM}I(Ml(gX|RVRrZ3rpRvtIJQIjqHrxGq3@EUs1MEy8! zpYFkZ@>XYVmqVHj1@GjdW?%@Gj5#V82Eei&6}RDb`kUGe#K)8x%)5916IC~qD5 zSnOawO7I5@6yplgMy{+5U^-qh_JaTrjtqDmt~EU16F<7N~0S`jIgB< ziuOw~36a=7MTj9p4Tg(AVMJCNk|m4-&K(AZ!pjQVt3;ASrC2K#c`EXueZ4qmpExuM z9>>7qh5fS2N^e88h`{AIw-%sa_OyFHI6tHDe!@s1UB$~+28Sy}2(5rEzJ2{LDdcE4 zrz???ki`=LasYyni}3MlOzE~$x~UrhJwx`X0aW8zK{MxCjA}e$Jc_7j@rd=3Y^r!u zm28SaoOsHAU`WrBNZI-K_`MZsE~hL-_)fv1J$dFizmc(7*}1OU&qQFIAkY zaCaUz5nt+3xQiut;Vi2@TX%r;48$H;>cRa0 zAgW`)gD1ew=^6=ZuHwyX*f%8J;H=q^$UrAyQ4mfCAVt)KAknCaEEb}D!ubU%I{q?j zeWB<&CEH>Lgd2@qI+V97M!VkD%;|aiWq{8-McL z5!`A|!sQ}|AWB`^0yenIqP@8)cqs0fEIta5loLPY#7E7Ep>HET_Trk#|Ayk@v5ITi zM0|IRBYC)l0%X3;CY;S~aW;#b&C*Radug7{4%h=HHtV(>XS453c9q9{S0+1og~^UM zlkr=m5+N1f!y~o>2;@$eGarCa0$3tq+nNBEGY2dM2Q0V;W9ZueOTdU4 z5Z|1?2`(bq8CWFaqNJ27*L}uC9~?e?b^O5uUqvU5*R(T+UoJ7x~?ui_}L&9UuE(}kZ< zkVo)pZ67ew_+1R*R|=H{l<%C~_T_K;&Teg&0APRgS@&g8$4fAUjE@@GH$Lf+KzU($}DzyJUM delta 7340 zcmeHM+iz6I8K0Rwd-ih9uGbg9V8ecAyug}}h&E&)4FMnS4k0Gw0!hPVbHh0{2{t5w z6gZX|T2fH~UZRZ}&Q=uK)G9{Lb9eLz(!MOCEs_syI& zHU;qqw4mjinRCu}`OSBm;qv9oH~*gTMg8D@DTI(;RSVDh$1~|Tj*rFS>`M*x!jZ-T zx#&K}ksT>9D=+h2N~Ps&CFXRd#N2sZ!b^1vF+U}IDP*ePDrTdUT#(_BaLcZ6Gm27J za-~$+LZDN=59b%i@DF)<2;vV_1*edskyJbaeQT4jGOFdc_{pzv0*sLC}i%sf@ zy4awetc&$(UtO$IgLSc1JyjQ>it3_I4b{aOHCz{~)zbZSai@B^E>@{$>Qbl!b+Jf| z)P*BCFSQpQ$Lv`_7{wKf89GU#93bOL{hVOkr-afh7`)G^2X|j9AZ6%QH`_@>u&t7wp999K9t$Dej-${ zP(@xMR55KZI}bWLfJrNU+vk5U6@de#CHbM;s?HTDcmno08&QkdtZu3Rt?aV&EKUMd})g#oi3X9IF_RoVW_-Ll)=d z7H8k$oSlaAi#Glem>)L=BrsRZuu!6}e6iiBc% zNIS^f2u=rSqt!;ILnW-;I656pFAh1^h|U4T4019gAZ-ZpY;-E4V%})sK&55^;f!w* zNTL~1A!~+DeVaCXtC%FDIRD0qfiov7c@4$7k(#C{ri@}9vh}j4A|_A&>J}&d&7xFS z>>SH*!9Syhbh-jNs;U}fm@Kusu`0;&Y#C}Gh#$C2M9{{h6gVh*B79FoHFZj1?9|jr zrfoI#B2%}f#*+2VB|oRO;hOq~8OG#d{5KwFDaW|f)Ms{PkgKW7c9yZvK9MRmEr%{` zV0>9V_M00NY!2uJ6L;Jfm_;bZ?SvP!^ZYG|Y6L2Rxe!>YsXxWcy}hRXz;td+{e>yM zVQIRnrY`Y3-HYcdQiFDQ3Bv=}yDYCC2zpc>s&zX21RI#Cs*3i(U+1~JVp<9mp7yMz zldnNPEOhE37wi5morC_Fn3d_Ep4Z^#Iqm>vfyj|yw&?*BWsM>(_Era|RYL4y#vK5w zILwa%6o$YtPXfZQl|M&20gv~{!5exWH#;q+Cn=Dj4XsH};Q7303EFQe(7eV6A9cn_ zUo*?P?`-E-lZ9ixcHn2wBl()f(?s4`RN1@lG&;HH#!}p*WH}15%*0oosb)>pp?N-( zLqU#pR?rd9ICvXSI+Tky;yEnVtahYl+AP7#@PimIg~4M#Cq`uK?Qu;+W?@$s*Y`BC zhQ+#@t0_5Kfc1ugWc0HvzygQutE$1kg)WNqp*R;h1K5*XcyuHvU?~p}p&5giiO8i4 zo6i!mlwnXkUNqfab1{3ZMSt|F&z;bTATJMsKBQpb&V_MmL}#Gq6{&d^kZ(#gNU>`I zUPT!LFx(tYI2cseIHLSk_Nh>m%-p(SkoU-E+!0yECA$GPLlG1Kzwt=dI4ip%%d_z0 ztkc3R;hV8QlOJIt3;o7(eAb)$DvusZh+mzViQiTH$*{gy$ zGn)-3<)%B!ws8LjTW)C@<62{|z3@tFjPd*{LfGEM@{5&trn0_O7QBtJ2_K`Z;lqvK z3{X7Azl6;J7~$C1*ymp{t$hp|;GCy8bLosZ>A1Y`8Hyuok6(dZaA20%Zn!H)R2Gg? zK>Rt44}W`G%y1hw%d(pxw_4pYA-oKgsrhcQ4aW4{+|(vs2EY-5WGehNa|AEy#M5q*))HH)Xw(=~cdF$Z+h zze3({l4L_}(>giAP94hF9uOHSmy1D$HyV*)z~h)_w8~Rzd*H2U94ll+XunIB3CqsR zWv?qJBS_%rUW783>_4(JV{BrQJCicS2_XN)*I@<@x%k?{d+y}RMj*MoUf^S%sY4gJ z?x6mBz!Zh*eWs{W?@3D8lMR(8f)u4u4pD^H<7jF81O~IY0>QkBA5?f(fD_kiY?BJ( zja^<%y~FHKr!O!?2KfLrt#2cKv#K8 zv(}mfY}q!8?aWU(PtsYd5V?V4?8u@!bGT-pB-t`bs9qF-Q7ngS9fkZ|@G6b~XP_Wp zh#TU#8#?{EyNk=@R($QidGq8BML-Vj%b~104J$oAqWM?=J8nhv0yc=#bdKN?Ia(0G zu{ddQlsP!Xi^^ceZpOZ`(=aNf z0H#T59Uv)GIDU3nq9Frv{oIfddNb=e984KoC%JfVmMPL~L?B+Wfsc!%NVlK_nQf#N zXg|&-a(iU@F+B%fW81Ph(;uRRXo;@(5Us?u*(Ny@7h4gc0p`Yl2wG*Sb(&JR^MX!( zaxFHcQF%f#Q_uTsH`PJ*+Z!X_%QxfC=)R zo$xJNwmI=J@!3Z>Q6(lMXDC|+;Na7^XTT!UWIj{(qzvT3zIdo&&M~X(S!HE2KV^_> zkbYj}^n-NzI#ZlezRR?Pzis?ObB==^r^=f8zBFkR&cSIG3Z^Mj!?g>jo1`-+#xxX-m9adWDye#$HFl+h&KaVz}83&tSOH4qw3#oj}-!mf7q9 z|FfFPr%*V`7~?d+Rf>mWtFH{#?Cx3@;jYuo=fZENp{$ z)4RJXo2g6euI(nvWpDxp?nye!4&Cajf4;=x5Z-P!6$&xCL?0&z0BBIsf6xJ$w|z^UVQY zpCY+za2~i4iaHlH!4s@*j>YK!v1}Em^2;na7{^+2#_CFjV}R23q(~!h-o_99Ja-3L zIiyI`|Kz^amIG#rnm9v}$)n~D8vKek5Z4GkZjX%vo6_2wIvi9;^ri-bg-i|vi&I`6E{4)(Q5HaUQ-s&Q>Pok0)+3-Gg9j%*OW6q7%AG3)i-O?F58Y zXyFOHcARaT@9lb}U79fS06T^F7cUaOF9vWF&maUz3yPiPi24g4E`z&pb`b`=iWt=sM9UaSgx z7Y99jqBU<*dAcXqe$(_ir#PGu=_i@;od;9ogfWSRtI+uOrS4d)8K;zK*Gv{c18%yu zX&sumUzzIA(xE_6I|@G8nmP=EY;h9FV3{3`VOX`p!z)rm5Ew4w4y|fCG~y1em;^gc z5qH7g`Ttpr=O9jSj!>q7;N}}O6x*2Pm`DLYH07qBpwuaJdv=9Wi@}!4jZL5SY&Dl1 ze9m~}mB}(CSIG+Dh&q`dWU`u-%H&J{+_0La#+}rrYJDbjb zZ FeedbackAction dislike_geometric(M& mlp, std::span current_out, float lr) noexcept { @@ -439,29 +440,36 @@ class FeedbackControllerCore : public FbStorage { const std::size_t neg_total = replay.negative_count(); const float avg_neg = replay.avg_negative_reward(); - FeedbackAction action; - if (pos_total == 0u) { - // 2. cold-start fallback (InterfaceRL.cpp:746): negative-LR - // training away from the heard action; no geometric push. The - // caller shows the "like a few sounds first" prompt. - mlp.train_targets(x_neg, a_neg, lr * 0.1f * avg_neg, focus_span_()); - action = FeedbackAction::GeometricColdStart; + // 2+3. ONE path, as upstream (InterfaceRL.tpp:696-761). With no likes + // stored there is no centroid to push away from, so the centroid is + // zeros and `have_positives=false` sends every dim in a random + // direction — upstream's `useRandom = !havePositives || ...`. We still + // report GeometricColdStart so the caller can show its "like a few + // sounds first" prompt, but the TRAINING is the same push at the same + // LR either way. (Until 2026-07-25 the cold start instead trained AWAY + // from the heard action at a NEGATIVE lr — a port of the older + // `0a541cc` fallback that upstream has since replaced.) + auto mean = this->centroid_buf(); + const bool have_positives = (pos_total > 0u); + if (have_positives) { + (void)replay.knn_positive_centroid(x_neg, kCentroidK, mean); } else { - // 3. centroid → target → train (InterfaceRL.cpp:602-743). - auto mean = this->centroid_buf(); - const std::size_t used = - replay.knn_positive_centroid(x_neg, kCentroidK, mean); - auto target = this->target_buf(); - compute_push_target(a_neg.subspan(0, (a_neg.size() < n_out) ? a_neg.size() : n_out), - std::span(mean.data(), n_out), - focus_span_(), geo_push_step(avg_neg), rng_, target); - const float ratio = geo_neg_lr_ratio(neg_total, pos_total); - mlp.train_targets(x_neg, std::span(target.data(), n_out), - lr * ratio, focus_span_()); - (void)used; - action = FeedbackAction::GeometricPush; + for (std::size_t j = 0; j < n_out; ++j) mean[j] = 0.f; } + auto target = this->target_buf(); + compute_push_target(a_neg.subspan(0, (a_neg.size() < n_out) ? a_neg.size() : n_out), + std::span(mean.data(), n_out), + focus_span_(), geo_push_step(avg_neg), have_positives, + rng_, target); + const float ratio = geo_neg_lr_ratio(neg_total, pos_total); + mlp.train_targets(x_neg, std::span(target.data(), n_out), + lr * ratio, focus_span_()); + + const FeedbackAction action = have_positives + ? FeedbackAction::GeometricPush + : FeedbackAction::GeometricColdStart; + // 4. decay + evict expired negatives (InterfaceRL.cpp:752-760). replay.decay_negatives(); diff --git a/nisps/ml/geo_push.hpp b/nisps/ml/geo_push.hpp index 364aa0e..077dca1 100644 --- a/nisps/ml/geo_push.hpp +++ b/nisps/ml/geo_push.hpp @@ -2,22 +2,32 @@ // dislike gesture (docs/adr/rl-feedback-design.md §2.1/§4). // // Verbatim port of the per-negative target computation in upstream -// InterfaceRL.cpp:713-735 (memllib @ 0a541cc): +// InterfaceRL.tpp:723-760 (memllib @ e291192 — vendored read-only at +// firmware/MEMLNaut-NISPS/lib/memllib/reference/): // -// pushStep = clamp(|avgRewardNeg|, 0.25, 1.0) * kGeometricPushScale -// dir[j] = neg_action[j] - meanPositiveAction[j] -// len = ||dir|| -// useRandom = (len <= 1e-4) (disliked ON the centroid) -// effectivePushStep = pushStep / (1 + len) (taper for far items) -// d = useRandom ? random ∈ [-1,1] : dir[j] / len -// target[j] = clamp(neg_action[j] + d * effectivePushStep, 0, 1) +// pushStep = clamp(|avgRewardNeg|, 0.25, 1.0) * kGeometricPushScale +// dir[j] = neg_action[j] - meanPositiveAction[j] (zeros when no likes) +// len = ||dir|| +// useRandom = !havePositives || (len <= 1e-4) +// d = useRandom ? random ∈ [-1,1] : dir[j] / len +// target[j] = clamp(neg_action[j] + d * pushStep, 0, 1) // inactive dims keep neg_action[j] // +// RE-BASED 2026-07-25 from `0a541cc` to `e291192`, where upstream had +// deliberately redesigned this and we had not noticed (the reference impl was +// out of tree — see reference/README.md). Three changes, all upstream's: +// kGeometricPushScale 0.5 -> 1.0, kNegLRBase 0.5 -> 1.5, and the `/(1+len)` +// TAPER DELETED. Upstream's own comment on the taper, at InterfaceRL.tpp:724: +// +// "No taper: a 'no' should clearly move the mapping away even from a sound +// already far from the liked region (the taper used to kill exactly that +// case). Bigger kGeometricPushScale + higher negLRRatio => the sound +// slides away faster/further." +// // SINGLE DELIBERATE FIRMWARE DIVERGENCE (recorded in ALIGNMENT.md): the // upstream `useRandom` branch draws libc `rand() & 0xFF`; we draw from the // caller's deterministic per-instance `nisps::Rng` so native == WASM parity -// holds. The branch only fires when a disliked action sits exactly on the -// centroid. +// holds. // // Pure free functions over spans — no replay/centroid logic in the MLP // kernel, no state, no heap (Anchor-First graft, ADR §0). @@ -34,10 +44,13 @@ namespace nisps::ml { -// Upstream InterfaceRL.hpp:293. -inline constexpr float kGeometricPushScale = 0.5f; +// Upstream InterfaceRL.hpp:409. +inline constexpr float kGeometricPushScale = 1.f; -// pushStep from the mean negative reward (InterfaceRL.cpp:713). +// Upstream InterfaceRL.hpp:410 — `negLRRatio = kNegLRBase - 0.4*negFraction`. +inline constexpr float kNegLRBase = 1.5f; + +// pushStep from the mean negative reward (InterfaceRL.tpp:733). inline float geo_push_step(float avg_reward_neg) noexcept { float mag = (avg_reward_neg < 0.f) ? -avg_reward_neg : avg_reward_neg; if (mag < 0.25f) mag = 0.25f; @@ -47,11 +60,17 @@ inline float geo_push_step(float avg_reward_neg) noexcept { // Compute the push-away target for ONE disliked action. `active_mask` // (1 = active) gates which dims move — empty ⇒ all active (this is the solo/ -// focus mask; upstream `activeDims_`). Writes n_out floats into `target`. +// focus mask; upstream `activeDims_`). `have_positives` is upstream's +// `posMemCount > 0`: with no likes stored yet there is no centroid to push +// away FROM, so every dim goes in a random direction instead (upstream passes +// an all-zero `meanPositiveAction` in that case, and `mean_positive` may be +// zeros here too — the flag, not the vector, is what selects the branch). +// Writes n_out floats into `target`. inline void compute_push_target(std::span neg_action, std::span mean_positive, std::span active_mask, float push_step, + bool have_positives, Rng& rng, std::span target) noexcept { const std::size_t n = neg_action.size(); @@ -62,8 +81,7 @@ inline void compute_push_target(std::span neg_action, len_sq += d * d; } const float len = std::sqrt(len_sq); - const bool use_random = (len <= 1e-4f); - const float effective = push_step / (1.0f + len); + const bool use_random = !have_positives || (len <= 1e-4f); for (std::size_t j = 0; j < n; ++j) { const bool active = @@ -75,21 +93,21 @@ inline void compute_push_target(std::span neg_action, const float d = use_random ? rng.next_float_signed() : ((neg_action[j] - mean_positive[j]) / len); - float t = neg_action[j] + d * effective; + float t = neg_action[j] + d * push_step; if (t < 0.f) t = 0.f; if (t > 1.f) t = 1.f; target[j] = t; } } -// Dynamic LR ratio (InterfaceRL.cpp:742-743): push harder when dislikes are +// Dynamic LR ratio (InterfaceRL.tpp:758-760): push harder when dislikes are // rare, gentler when they flood the buffer. inline float geo_neg_lr_ratio(std::size_t neg_count, std::size_t pos_count) noexcept { const std::size_t total = neg_count + pos_count; const float neg_fraction = (total > 0u) ? static_cast(neg_count) / static_cast(total) : 0.f; - return 0.5f - 0.4f * neg_fraction; + return kNegLRBase - 0.4f * neg_fraction; } } // namespace nisps::ml diff --git a/tests/cpp/test_mlp_geo_dislike.cpp b/tests/cpp/test_mlp_geo_dislike.cpp index 9e42852..f8cb4df 100644 --- a/tests/cpp/test_mlp_geo_dislike.cpp +++ b/tests/cpp/test_mlp_geo_dislike.cpp @@ -149,20 +149,50 @@ NISPS_TEST(geo_push_target_moves_away_from_centroid) { neg[j] = 0.6f; mean[j] = 0.4f; // dir = +0.2 per dim → push increases values } - const float step = nisps::ml::geo_push_step(-1.f); // clamp(1,0.25,1)*0.5 = 0.5 - NISPS_EXPECT_NEAR(step, 0.5f, 1e-7); - nisps::ml::compute_push_target(neg, mean, {}, step, rng, target); + const float step = nisps::ml::geo_push_step(-1.f); // clamp(1,0.25,1)*1.0 = 1.0 + NISPS_EXPECT_NEAR(step, 1.f, 1e-7); + nisps::ml::compute_push_target(neg, mean, {}, step, /*have_positives=*/true, rng, target); for (std::size_t j = 0; j < kNOut; ++j) { NISPS_EXPECT(target[j] > neg[j]); // strictly away from the centroid NISPS_EXPECT(target[j] <= 1.f); } - // Taper: a far-away negative moves LESS than a near one for the same step. - std::array mean_far{}; - std::array target_far{}; - for (std::size_t j = 0; j < kNOut; ++j) mean_far[j] = 0.0f; // larger len - nisps::ml::compute_push_target(neg, mean_far, {}, step, rng, target_far); - NISPS_EXPECT((target_far[0] - neg[0]) < (target[0] - neg[0])); + // NO TAPER (upstream e291192, InterfaceRL.tpp:724): distance from the + // liked centroid must NOT shrink the push. The direction is a unit + // vector either way, so a far-away negative is displaced exactly as far + // as a near one — which is the case the deleted /(1+len) used to kill. + // (Both are clamped at 1.0 here, so compare an unsaturated dim.) + std::array near_neg{}, far_mean{}, near_mean{}; + std::array t_near{}, t_far{}; + for (std::size_t j = 0; j < kNOut; ++j) { + near_neg[j] = 0.5f; + near_mean[j] = 0.49f; // len ~= 0.024 across 6 dims + far_mean[j] = 0.0f; // len ~= 1.22 + } + nisps::ml::compute_push_target(near_neg, near_mean, {}, 0.1f, true, rng, t_near); + nisps::ml::compute_push_target(near_neg, far_mean, {}, 0.1f, true, rng, t_far); + NISPS_EXPECT_NEAR(t_far[0] - near_neg[0], t_near[0] - near_neg[0], 1e-6); +} + +// With nothing liked yet there is no centroid to push away from, so upstream +// pushes in a RANDOM direction per dim rather than doing nothing +// (`useRandom = !havePositives || ...`, InterfaceRL.tpp:745). +NISPS_TEST(geo_push_target_random_direction_when_no_positives) { + nisps::Rng rng(11ull); + std::array neg{}, mean{}, target{}; + for (std::size_t j = 0; j < kNOut; ++j) { neg[j] = 0.5f; mean[j] = 0.5f; } + + nisps::ml::compute_push_target(neg, mean, {}, 0.25f, /*have_positives=*/false, + rng, target); + bool any_moved = false, any_down = false, any_up = false; + for (std::size_t j = 0; j < kNOut; ++j) { + if (target[j] != neg[j]) any_moved = true; + if (target[j] < neg[j]) any_down = true; + if (target[j] > neg[j]) any_up = true; + NISPS_EXPECT(target[j] >= 0.f && target[j] <= 1.f); + } + NISPS_EXPECT(any_moved); + NISPS_EXPECT(any_down && any_up); // per-dim signs, not one global direction } NISPS_TEST(geo_push_respects_active_mask_and_clamps) { @@ -175,7 +205,8 @@ NISPS_TEST(geo_push_respects_active_mask_and_clamps) { mean[j] = 0.01f; } std::array mask{1u, 0u, 1u, 0u, 1u, 0u}; - nisps::ml::compute_push_target(neg, mean, mask, 0.5f, rng, target); + nisps::ml::compute_push_target(neg, mean, mask, 0.5f, /*have_positives=*/true, + rng, target); for (std::size_t j = 0; j < kNOut; ++j) { if (mask[j]) { NISPS_EXPECT(target[j] >= neg[j]); // pushed (and clamped at 1) @@ -197,27 +228,30 @@ NISPS_TEST(geo_dislike_cold_start_then_push) { set_inputs(m, 0.25f, 0.75f); m.process(); - // Cold start: no positives yet → negative-LR fallback. + // Cold start: no positives yet. Since the 2026-07-25 re-base onto + // upstream e291192 this is NOT a separate inert branch — it is the same + // push, in a random direction because there is no centroid to move away + // from. So a 'no' moves the mapping even before anything is liked, which + // is exactly what upstream's comment at InterfaceRL.tpp:724 argues for. auto before = m.get_weights(); std::array snap{}; for (std::size_t i = 0; i < snap.size(); ++i) snap[i] = before[i]; - // With the heard action == the net's own output the MSE derivative is - // zero, so the fallback is INERT — the conservative cold start the ADR - // mandates (never destabilise before any positives exist). const FeedbackAction a1 = fb.on_down(m, {}, 0.1f, 0.5f, {}); NISPS_EXPECT(a1 == FeedbackAction::GeometricColdStart); NISPS_EXPECT(fb.negative_count() == 1u); + bool moved = false; { auto after = m.get_weights(); for (std::size_t i = 0; i < snap.size(); ++i) { - NISPS_ASSERT(after[i] == snap[i]); + if (after[i] != snap[i]) { moved = true; break; } } } + NISPS_EXPECT(moved); - // When the HEARD action differs from the net's raw output (the real - // browser/firmware case — the user hears the post-pipeline vector), the - // negative-LR fallback trains AWAY from it: weights move. + // The same holds when the HEARD action differs from the net's raw output + // (the real browser/firmware case — the user hears the post-pipeline + // vector), which used to be the ONLY case that moved anything. std::array heard{}; { auto outs = m.outputs(); @@ -227,14 +261,6 @@ NISPS_TEST(geo_dislike_cold_start_then_push) { } const FeedbackAction a1b = fb.on_down(m, heard, 0.1f, 0.5f, {}); NISPS_EXPECT(a1b == FeedbackAction::GeometricColdStart); - bool moved = false; - { - auto after = m.get_weights(); - for (std::size_t i = 0; i < snap.size(); ++i) { - if (after[i] != snap[i]) { moved = true; break; } - } - } - NISPS_EXPECT(moved); // Feed positives via the like path, then dislike → geometric push. set_inputs(m, 0.2f, 0.2f);