Skip to content

nmmo3: observation encoder never clears entity bytes — stale "live enemy" claims persist indefinitely #629

Description

@daveey

Affected file: ocean/nmmo3/nmmo3.hcompute_all_obs()
Pinned lines (default branch 4.0, commit c5d3c637):

int pid = env->pids[map_adr];
if (pid != -1) {
Entity* seen = get_entity(env, pid);
env->observations[obs_adr+4] = seen->type;
env->observations[obs_adr+5] = seen->element;
int delta_comb_obs = (seen->comb_lvl - comb_lvl) / 2;
if (delta_comb_obs < 0) {
delta_comb_obs = 0;
}
if (delta_comb_obs > 4) {
delta_comb_obs = 4;
}
env->observations[obs_adr+6] = delta_comb_obs;
env->observations[obs_adr+7] = seen->hp / 20; // Bucketed for discrete
env->observations[obs_adr+8] = seen->anim;
env->observations[obs_adr+9] = seen->dir;
}

(allocation-only zeroing at line 797)

Illustrated walkthrough (captured frames, byte timelines, camera-drag demo, rendered):
https://htmlpreview.github.io/?https://gist.githubusercontent.com/daveey/8d0e57d5c584275a65f14ab725973608/raw/nmmo3-obs-residue-bug.html

Summary

The per-agent observation buffer is calloc'd once at allocate_mmo() and never
cleared again. Each tick, compute_all_obs() rewrites every window cell's terrain
bytes (0–1) and item bytes (2–3) unconditionally, but writes the six entity
bytes (4–9: type, element, level-delta, hp-bucket, anim, dir) only inside
if (pid != -1)there is no else branch. When an entity leaves a cell, its
bytes remain in the buffer indefinitely.

Consequences:

  1. Observations assert false state. Cells that are empty in the world claim a
    live enemy of a specific type/level/HP, with no timestamp, tombstone, or any
    signal distinguishing residue from truth.
  2. Phantoms accumulate. In a capture with a stationary observer, a single
    wandering NPC painted 23 stale "live enemy" claims into the 11×15 window
    within 100 ticks; individual stale cells persisted unchanged for 200+ ticks
    (unbounded). With 2048 NPCs on 512², every agent's window smears continuously.
  3. Stale claims are camera-anchored. The buffer is indexed by window slot, so
    when the observer moves, stale bytes keep their window position — their implied
    world position translates with the observer. (Verified: after the observer
    walked 3 tiles, all 23 phantom claims re-mapped to new world cells, bit-identical.)
  4. No liveness bit exists. A live entity re-written with byte-identical values
    produces the same bits as a never-rewritten stale cell, so consumers cannot
    distinguish "still there, unchanged" from "long gone" by frame-differencing.

Reproduction (upstream code only)

The invariant to test: a window cell's entity-type byte should be nonzero iff an
entity currently occupies that world cell.

  1. Init the env with any seed; feed one agent NOOP every tick (fixed window).
  2. Each tick after compute_all_obs(), for each of that agent's window cells,
    compare env->pids[map_offset(env, r, c)] against obs byte 4 (type).
  3. Assert type == 0 ⟺ pid == -1. This fails within the first few hundred ticks
    on every seed — as soon as any NPC walks through the window and leaves.
  4. Log one violating cell: its bytes persist unchanged indefinitely. Then move the
    agent and observe the stale slot re-mapping to a different world cell.

Example captured frames (stationary observer, seed 1; E = obs claim confirmed by
ground truth, R = obs claims a live enemy where the world has none, @ = agent):

tick 1                tick 51                tick 101
...............       RRRR....RRRRRRR        RRRR.R..RRRRRRR
.............ER       ......RRR....RR        .....RRRR....RR
...............       ...............        .....R.........
...............       ...............        .....RR........
...............       ...............        ......RR.......
.......@.......       .......@.......        .......@.......
                      (0 real enemies,       (0 real enemies,
                       16 stale claims)       23 stale claims)

Byte timeline of one stale cell — [type, element, delta, hp/20, anim, dir],
ground truth EMPTY at every row: [2,2,4,4,0,0] at ticks 1–26, [2,2,4,4,1,0] at
ticks 51–201+ (the anim flip shows the cell was silently re-stamped and re-frozen —
there is no way to know when bytes were last written).

Impact on trained policies

We measured the fix's effect on the bundled pretrained demo policy
(nmmo3_weights.bin) with a deterministic interleaved-seat benchmark (8 seeds,
same episodes both arms):

metric residue encoding fixed encoding
pretrained net, mean seat score @1500t 7.18 3.17 (−56%)
pretrained net, mean seat score @5000t 7.11 2.70 (−62%)
pretrained net, deaths/agent @5000t 4.4 16.4 (×3.7)
a scripted reference agent @5000t 2.03 2.28 (+12%)

The trained network has learned to read the stale bytes as a free last-seen-enemy
memory channel; with the leak removed it forgets enemies that leave a cell and
walks into the weakest melee NPCs (deaths to the lowest tier: 7 → 81 @5000t).
Any checkpoint trained on the current encoding will regress hard on a fixed
encoding
— a fix should probably ship behind a config flag or an env version
bump, and the demo weights would need retraining.

Suggested fix

                 int pid = env->pids[map_adr];
                 if (pid != -1) {
                     /* ... existing writes of bytes 4..9 ... */
                     env->observations[obs_adr+9] = seen->dir;
-                }
+                } else {
+                    env->observations[obs_adr+4] = 0;
+                    env->observations[obs_adr+5] = 0;
+                    env->observations[obs_adr+6] = 0;
+                    env->observations[obs_adr+7] = 0;
+                    env->observations[obs_adr+8] = 0;
+                    env->observations[obs_adr+9] = 0;
+                }
                 obs_adr += 10;

(or memset the agent's obs block at the top of compute_all_obs).

Backward-compatible variant (probably the friendliest): make truthful entity
bytes opt-in per agent/policy — a per-agent flag (config or API) that gates the
else-clear for that agent's window only. Non-opted agents take the exact legacy
code path, so their observations stay bit-identical and existing checkpoints
(including the bundled demo weights) are untouched; new consumers opt in and get
snapshot semantics. A richer flavor of the same idea: keep the residue (it is
genuinely useful last-seen memory) and, for opted-in agents, set a freshness bit
(e.g. type | 0x80) on cells written this tick and clear it otherwise — truth and
memory both available, explicitly distinguished, still bit-identical for everyone
who doesn't ask.

Happy to open a PR with any of these variants.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions