From 4cd5618435d476c19d27cf31a4fe2bf383dbae06 Mon Sep 17 00:00:00 2001 From: Davide Cappellini Date: Mon, 21 Sep 2026 06:56:44 +0200 Subject: [PATCH] fix(test): repair the flaky offline==online acceptance; make the tie-break truly random TASK 2 - THE FLAKY ACCEPTANCE TEST, root-caused. It was NOT a live/offline boundary race as suspected. The replay spawned gun 13 (TMSelect) while live has EnableTmSelector = false and never does. The shared VirtualTracker ring is ORDER-SENSITIVE, so gun 13's extra 4 bullets/tick shift the ring head and permute the per-tick RESOLUTION ORDER of every other gun. The learning guns append observations in resolution order, so their predictions shifted and produced small hit deltas that moved between runs. Evidence: the first KNN divergence was at rtick=174 with the SAME resolution set merely reordered (live ft133,138,139,142,148,150,151 vs offline ft150,151,133,138,139,142,148); after closing gun 13's ready gate offline the live and offline KNN traces became BYTE-IDENTICAL (diff empty, 904/904 lines). Fix: mirror the live rack in the replay. No tick exclusion, no tolerance loosening. Stability: 5/5 consecutive runs now report 12/12 exact, each with enemyDied=true - the death boundary is included, not excluded. The proof is now real rather than a lucky run. TASK 1 - the tie-break was not random. randomize() was only reached incidentally through initTsetlinGun(), so a rack without Tsetlin had a fixed rand() stream and ties always resolved the same way across process restarts. Added seedSelectorRng() after gun construction, honouring GUN_SELECTOR_SEED. Evidence: unseeded, 6 separate processes gave different pick sequences; with GUN_SELECTOR_SEED=42, 3 processes gave identical sequences. TASK 3 - PRUNING DOES NOT HELP; keep the full rack. 15 PAIRED runs per variant vs DrussGT, 8 rounds, identical seeds: baseline 3238 shots 6.18% (events 6.16%) 200 dmg/run Tsetlin disabled 3522 shots 5.76% (events 5.71%) 197 dmg/run Tsetlin+Displace 3478 shots 5.46% (events 5.37%) 183 dmg/run Paired permutation tests: -0.34pp p=0.57 and -0.70pp p=0.21. Per-run distributions completely overlap (baseline range [2.68, 10.00]; 15/15 and 14/15 runs inside it). A Crazy control showed no separation either. So removing the measured-worst real performers is neutral-to-slightly-negative, and with sd ~1.8pp a definitive claim either way would need far more runs. CORRECTION TO A CLAIM I MADE: the 'virtual metric is INVERTED' finding does NOT reproduce. Job-24 measured Spearman -0.374; this job measures +0.335 over the same 13 guns with a different but equally defensible aggregation. Two opposite signs means the correlation is NOT robustly negative - it is WEAK AND SIGN-UNSTABLE. The honest statement is that virtual hit rate is a poor ranker, not an inverted one. The docs assert the inversion and need correcting. Also adds per-process GUN_STATS_PATH/GUN_SHOTLOG_PATH so concurrent A/B runs do not clobber each other, and an env-gated GUN_RACK_DISABLE for rack A/Bs. All default behaviour is unchanged when the env vars are unset. --- ModularBot_garage/src/ModularBot.nim | 70 ++++++++++++++----- .../tests/acceptance_offline_vs_online.nim | 2 +- common_libs/tests/range_guns.nim | 15 +++- 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/ModularBot_garage/src/ModularBot.nim b/ModularBot_garage/src/ModularBot.nim index 46cdfc6..a8ba324 100644 --- a/ModularBot_garage/src/ModularBot.nim +++ b/ModularBot_garage/src/ModularBot.nim @@ -3,7 +3,7 @@ ## Radar: RadarLockModule (1v1) / MeleeScanModule (2+ enemies), auto-switched per tick. ## Movement: OscillatorModule (perpendicular strafing). -import std/[math, os, strformat, tables, sets, json] +import std/[math, os, strformat, tables, sets, json, random, strutils] import robocode_tankroyale_botapi import radar_harness/radar_interface import radars/radar_lock_module @@ -55,6 +55,25 @@ const ShotLog = true ## can enable recording for just the battle it spawns by exporting the env var. let RecordWorldState* = existsEnv("TR_RECORD_WORLDSTATE") const WorldStateRecordPath = "/tmp/worldstate_record.jsonl" +## Runtime rack pruning for MEASURED A/B runs: comma-separated gun ids to remove +## from the virtual-bullet rack. A disabled gun never spawns virtual bullets, so +## its fitness window stays empty and the selector can never pick it (chooseFromFit +## skips guns with no observations). Read once at process start so a SINGLE frozen +## binary can be A/B'd by exporting GUN_RACK_DISABLE. Empty/unset = full rack. +let DisabledGuns = + block: + var s: HashSet[int] + for tok in getEnv("GUN_RACK_DISABLE", "").split(','): + let t = tok.strip() + if t.len > 0: + try: s.incl parseInt(t) + except ValueError: discard + s + +proc gunDisabled(id: int): bool {.inline.} = id in DisabledGuns +## Per-process output paths so concurrent A/B runs do not clobber each other. +let GunStatsPath = getEnv("GUN_STATS_PATH", "/tmp/gun_stats.jsonl") +let ShotLogPath = getEnv("GUN_SHOTLOG_PATH", "/tmp/shot_log.jsonl") const GunNames = ["HeadOn", "Linear", "Tsetlin", "Circular", "GuessFactor", "Pattern", "WallBounce", "Accel", "StopShot", "Displace", "AvgLead", "DecayGF", "KNN", "TMSelect"] const @@ -145,7 +164,7 @@ proc writeShotLog(shot: PendingShot, hit: bool, unresolved: bool) = "unresolved": unresolved } try: - let f = open("/tmp/shot_log.jsonl", fmAppend) + let f = open(ShotLogPath, fmAppend) f.writeLine($row) f.close() except CatchableError: @@ -369,7 +388,7 @@ method onRoundEnded*(bot: ModularBot, e: RoundEndedEventForBot) = "vDropped": bot.tracker.droppedBullets, "vStarved": bot.guessFactor.waveStarved + bot.decayGF.waveStarved + bot.knnGun.waveStarved } - let f = open("/tmp/gun_stats.jsonl", fmAppend) + let f = open(GunStatsPath, fmAppend) f.writeLine($row) f.close() # Task A: shots still in flight at round end never resolved, so count them as @@ -619,20 +638,20 @@ method run*(bot: ModularBot) = tmselPreds[i] = bot.tmSelector.predict(bot.lastState, bulletSpeed(PowerBins[i])) bot.tracker.spawnBullets(0, headsUp, bot.lastState, tid) - bot.tracker.spawnBullets(1, linPreds, bot.lastState, tid) - if bot.tsetlin.isWarmedUp(): + if not gunDisabled(1): bot.tracker.spawnBullets(1, linPreds, bot.lastState, tid) + if not gunDisabled(2) and bot.tsetlin.isWarmedUp(): bot.tracker.spawnBullets(2, tmPreds, bot.lastState, tid) - bot.tracker.spawnBullets(3, circPreds, bot.lastState, tid) - bot.tracker.spawnBullets(4, gfPreds, bot.lastState, tid) - bot.tracker.spawnBullets(5, pmPreds, bot.lastState, tid) - bot.tracker.spawnBullets(6, wbPreds, bot.lastState, tid) - bot.tracker.spawnBullets(7, acPreds, bot.lastState, tid) - bot.tracker.spawnBullets(8, ssPreds, bot.lastState, tid) - bot.tracker.spawnBullets(9, dsPreds, bot.lastState, tid) - bot.tracker.spawnBullets(10, alPreds, bot.lastState, tid) - bot.tracker.spawnBullets(11, dgPreds, bot.lastState, tid) - bot.tracker.spawnBullets(12, knnPreds, bot.lastState, tid) - if EnableTmSelector and bot.tmSelector.isWarmedUp(): + if not gunDisabled(3): bot.tracker.spawnBullets(3, circPreds, bot.lastState, tid) + if not gunDisabled(4): bot.tracker.spawnBullets(4, gfPreds, bot.lastState, tid) + if not gunDisabled(5): bot.tracker.spawnBullets(5, pmPreds, bot.lastState, tid) + if not gunDisabled(6): bot.tracker.spawnBullets(6, wbPreds, bot.lastState, tid) + if not gunDisabled(7): bot.tracker.spawnBullets(7, acPreds, bot.lastState, tid) + if not gunDisabled(8): bot.tracker.spawnBullets(8, ssPreds, bot.lastState, tid) + if not gunDisabled(9): bot.tracker.spawnBullets(9, dsPreds, bot.lastState, tid) + if not gunDisabled(10): bot.tracker.spawnBullets(10, alPreds, bot.lastState, tid) + if not gunDisabled(11): bot.tracker.spawnBullets(11, dgPreds, bot.lastState, tid) + if not gunDisabled(12): bot.tracker.spawnBullets(12, knnPreds, bot.lastState, tid) + if EnableTmSelector and not gunDisabled(13) and bot.tmSelector.isWarmedUp(): bot.tracker.spawnBullets(13, tmselPreds, bot.lastState, tid) # Build slim enemy table for tickBullets @@ -735,6 +754,22 @@ method run*(bot: ModularBot) = setGunTurnRate(normDelta) +proc seedSelectorRng() = + ## Seed the process-global RNG exactly once at bot startup so the gun + ## selector's "random" tie-break (`chooseFromFit` -> rand) actually varies + ## across process restarts. Previously the only `randomize()` call reached on + ## the live path was incidental, inside the Tsetlin gun's constructor, so a + ## rack without Tsetlin produced a FIXED tie-break sequence forever. + ## + ## `GUN_SELECTOR_SEED`, when set to an integer, pins the stream so A/B runs are + ## reproducible; otherwise time+pid seeding makes each process independent. + let s = getEnv("GUN_SELECTOR_SEED", "") + if s.len > 0: + try: randomize(parseInt(s.strip())) + except ValueError: randomize() + else: + randomize() + when isMainModule: var bot = ModularBot( tracker: vb.initTracker(14), # 0: HeadOn, 1: Linear, 2: Tsetlin, 3: Circular, 4: GuessFactor, 5: Pattern, 6: WallBounce, 7: Accel, 8: StopShot, 9: Displace, 10: AvgLead, 11: DecayGF, 12: KNN, 13: TMSelect @@ -759,4 +794,7 @@ when isMainModule: moveTracker: mvb.initVirtualBodyTracker(1), currentGun: -1, ) + # Must run AFTER the gun constructors (Tsetlin/TMSelector call randomize() + # unconditionally) so the explicit seed override is the value that survives. + seedSelectorRng() start(bot, botJsonPath) diff --git a/common_libs/tests/acceptance_offline_vs_online.nim b/common_libs/tests/acceptance_offline_vs_online.nim index 76bb729..c3a405b 100644 --- a/common_libs/tests/acceptance_offline_vs_online.nim +++ b/common_libs/tests/acceptance_offline_vs_online.nim @@ -78,7 +78,7 @@ proc main() = let online = lastOnlineRound(statsPath) let fx = loadFixture(recordPath) - let reports = replayFixture(fx, buildAllGunDrivers(), liveActual = true) + let reports = replayFixture(fx, buildAllGunDrivers(enableTmSelector = false), liveActual = true) # Map online stats by gun id. var onShots: array[14, int] diff --git a/common_libs/tests/range_guns.nim b/common_libs/tests/range_guns.nim index 9138f63..18d2afc 100644 --- a/common_libs/tests/range_guns.nim +++ b/common_libs/tests/range_guns.nim @@ -19,12 +19,21 @@ import guns/decay_gf import guns/knn_gun import guns/tm_selector -proc buildAllGunDrivers*(seed = -1): seq[GunDriver] = +proc buildAllGunDrivers*(seed = -1, enableTmSelector = true): seq[GunDriver] = ## seed >= 0 re-seeds the global RNG after constructing the stochastic guns ## (Tsetlin and the TM selector both call randomize() in their constructors), ## so their learning is reproducible for offline runs. ## ## Order matches ModularBot's gun ids exactly (TMSelect appended at 13). + ## + ## `enableTmSelector` must MIRROR the live rack. The shipped ModularBot has + ## `EnableTmSelector = false`, so the live loop never spawns gun-13 virtual + ## bullets. The replay's shared VirtualTracker ring is order-sensitive: extra + ## gun-13 spawns shift the ring head and permute the per-tick resolution ORDER + ## of every other gun, which scrambles the `obs` insertion order of the + ## learning guns (KNN, DecayGF) and makes the offline metric diverge from the + ## live one. Callers that mirror the shipped bot must pass false (the + ## acceptance test does); tests that specifically exercise TMSelect pass true. var tsetlin = initTsetlinGun() var tmSelector = initTmSelectorGun() if seed >= 0: @@ -45,6 +54,10 @@ proc buildAllGunDrivers*(seed = -1): seq[GunDriver] = makeDriver("KNN", initKNNGun()), makeDriver("TMSelect", tmSelector), ] + if not enableTmSelector: + ## Same effect as the live `if EnableTmSelector` gate: never spawn gun 13. + ## Keep the slot so gun ids / report indices are unchanged. + result[13].readyCb = proc(): bool = false proc makeTsetlinDriver*(seed = -1): tuple[driver: GunDriver, gun: ref TsetlinGun] = ## Same as makeDriver("Tsetlin", ...) but keeps a handle to the concrete gun,