diff --git a/detail_probe.py b/detail_probe.py index dfc35fb8..0793616c 100644 --- a/detail_probe.py +++ b/detail_probe.py @@ -77,14 +77,46 @@ def _log(): return fl +_ENV_SWITCH = "FUNPACK_DETAIL_PROBE" + + +def _switch_path(): + base_dir = os.path.dirname(os.path.abspath(__file__)) + return os.path.join(base_dir, "refinements", "detail_probe", "enabled") + + def collection_enabled(): - """Rides the block-influence switch -- both are research measurement, opted into from - the same place, and a second toggle for the same decision is a worse UI, not a safer one.""" + """Its own switch, NOT the block probe's, despite both being research measurement. + + They cost wildly different amounts and that difference is the point: the block probe + gathers ~17k x 5120 rows per block per step, while this copies ONE latent once per run. + Allocator churn on that scale can change which GPU kernels get selected, so a probe can + move the numbers it is measuring -- and answering "is the measurement causing the + variation?" requires running this one WITHOUT that one. A single shared toggle made that + experiment impossible to perform.""" + raw = os.environ.get(_ENV_SWITCH, "").strip().lower() + if raw: + return raw in ("1", "true", "yes", "on") try: - from . import block_influence as bi - except ImportError: - import block_influence as bi - return bi.collection_enabled() + with open(_switch_path(), "r", encoding="utf-8") as fh: + return fh.read().strip() == "1" + except (OSError, ValueError): + return False + + +def set_collection_enabled(on): + on = bool(on) + os.environ[_ENV_SWITCH] = "1" if on else "0" + try: + os.makedirs(os.path.dirname(_switch_path()), exist_ok=True) + tmp = _switch_path() + ".tmp" + with open(tmp, "w", encoding="utf-8") as fh: + fh.write("1" if on else "0") + os.replace(tmp, _switch_path()) + except OSError as e: + _log().failed("H3 detail probe", "switch save", e, + "recording is set for this session only and reverts after a restart") + return on def state_path(refinement_key): diff --git a/movie_editor/frontend/api.js b/movie_editor/frontend/api.js index 52f4b368..dc3438eb 100644 --- a/movie_editor/frontend/api.js +++ b/movie_editor/frontend/api.js @@ -236,6 +236,7 @@ URL.revokeObjectURL(url); }, detailProbeStatus: (key) => j("GET", API("/detail_probe") + `?key=${encodeURIComponent(key || "default")}`), + detailProbeSetEnabled: (key, enabled) => j("POST", API("/detail_probe"), { key: key || "default", enabled: !!enabled }), detailProbeClear: (key) => j("POST", API("/detail_probe/clear"), { key: key || "default" }), // Served by ComfyUI's own same-origin /view endpoint — no editor route needed. diff --git a/movie_editor/frontend/settings_window.js b/movie_editor/frontend/settings_window.js index bc5f163b..a201eb0e 100644 --- a/movie_editor/frontend/settings_window.js +++ b/movie_editor/frontend/settings_window.js @@ -401,13 +401,26 @@ // only on explicit request (the "Read" button below). let reinsState = null, reinsSweep = null, reinsSweepBusy = false, reinsError = ""; let biState = null, biError = ""; - let dpRows = null, dpError = ""; + let dpRows = null, dpError = "", dpState = null; // Reads a comparison the way the user would judge it by eye, but from three numbers that // can disagree with each other -- that disagreement is the whole point. Detail up on // existing edges with the picture intact is sharpening; detail up spread evenly is grain; // structure down means it is simply a different generation. - function dpVerdict(r) { + // How much the picture moves between two runs that changed NOTHING. Measured from your own + // rows rather than assumed: on a deterministic rig it is ~0, and on this one it is ~0.10 + // (visible as eyes/mouth differing while the motion stays put). Without it a structure of + // 0.58 is a number; with it, it is "6x the noise", which is the part that means something. + function dpNoiseFloor(rows) { + const quiet = (rows || []).filter((r) => r.changed + && !Object.keys(r.changed).length && r.same_seed !== false) + .map((r) => 1 - r.structure) + .sort((a, b) => a - b); + if (!quiet.length) return null; + return quiet[Math.floor(quiet.length / 2)]; // median, so one outlier cannot set it + } + + function dpVerdict(r, floor) { // A low `structure` has two very different causes and the numbers cannot separate them, // so the seed decides. Different seeds = two different generations and the comparison // never was an A/B (a reference pins the subject, not the sample). Same seed = the @@ -421,17 +434,16 @@ const onlySeed = r.changed && !Object.keys(r.changed).filter((k) => k !== "seed").length; if (onlySeed && r.same_seed) { // Nothing differed at all: this is the instrument's noise floor for this setup. - return r.structure > 0.98 && Math.abs(r.detail - 1) < 0.02 - ? `stable — two identical runs match (structure ${r.structure.toFixed(3)}), so a real ` - + "A/B can be trusted" - : `two identical runs already differ this much (structure ${r.structure.toFixed(2)}, ` - + `detail x${r.detail.toFixed(2)}) — generation is not reproducible here, so no A/B ` - + "on this setup means anything"; + return `noise floor: two identical runs differ by ${(moved * 100).toFixed(0)}% ` + + `(detail x${r.detail.toFixed(2)}). Anything smaller than this is not a result.`; } + const moved = 1 - r.structure; + const vsFloor = floor && floor > 1e-6 ? ` — ${(moved / floor).toFixed(1)}x your noise floor` + : ""; if (r.structure < 0.85) { return r.same_seed - ? `the shot itself moved (structure ${r.structure.toFixed(2)}) — this changed the ` - + "picture, not just its detail" + ? `the shot moved ${(moved * 100).toFixed(0)}%${vsFloor} — this changed the picture, ` + + "not just its detail" : "different seed — two separate generations, so detail cannot be compared. Rerun " + "the same seed with the change off, then on."; } @@ -475,11 +487,37 @@ clear(box); const rows = el("div", "sw-rows"); const n = dpRows ? dpRows.length : 0; + + const row = el("div", "sw-row"); + const main = el("div", "sw-row-main"); + main.append(el("div", "sw-row-title", "Score each run against the one before it")); + main.append(el("div", "sw-row-hint", + "Off by default. Recording only. Separate from the block probe on purpose — this " + + "copies one latent per run, that one reads every block on every step, so leaving " + + "this on with that one off is how you check whether the heavy probe is itself " + + "moving the picture.")); + row.append(main); + const lbl = el("label", "chk es-toggle"); + const cb = el("input"); + cb.type = "checkbox"; + cb.checked = !!(dpState && dpState.enabled); + cb.onchange = async () => { + dpError = ""; + try { + const r = await window.MovieEditorAPI.detailProbeSetEnabled(key, cb.checked); + dpState = r; dpRows = r.rows || dpRows; + } catch (e) { dpError = String(e.message || e); } + paint(); + }; + lbl.append(cb, el("span", null, "")); + row.append(lbl); + rows.append(row); + rows.append(actionRow("Start fresh", "Throws away every recorded comparison for this key.", "Clear…", async () => { dpError = ""; - try { const r = await window.MovieEditorAPI.detailProbeClear(key); dpRows = r.rows; } + try { const r = await window.MovieEditorAPI.detailProbeClear(key); dpState = r; dpRows = r.rows; } catch (e) { dpError = String(e.message || e); } paint(); }, { disabled: !n, danger: true })); @@ -494,11 +532,12 @@ return; } const tbl = el("div", "sw-rows"); + const floor = dpNoiseFloor(dpRows); // Newest first: the run you just made is the one you are asking about. dpRows.slice().reverse().forEach((r) => { tbl.append(infoRow( dpChangeSummary(r), - dpVerdict(r), + dpVerdict(r, floor), r.same_seed !== false && r.structure >= 0.85 && r.detail > 1.02 && r.edge_aligned > 0.35)); const seeds = r.seed_before == null && r.seed_after == null ? "" @@ -512,7 +551,7 @@ paint(); if (window.MovieEditorAPI) { window.MovieEditorAPI.detailProbeStatus(key) - .then((r) => { dpRows = r.rows || []; paint(); }) + .then((r) => { dpState = r; dpRows = r.rows || []; paint(); }) .catch(() => {}); } return box; diff --git a/movie_editor/server.py b/movie_editor/server.py index ed833f6c..dcf5cd0b 100644 --- a/movie_editor/server.py +++ b/movie_editor/server.py @@ -2268,9 +2268,24 @@ if web is not None and PromptServer is not None: async def _detail_status(req): key = str(req.rel_url.query.get("key") or "default").strip() or "default" try: - return web.json_response({"key": key, "rows": _detail_module().rows(key)}) + dp = _detail_module() + return web.json_response({"key": key, "rows": dp.rows(key), + "enabled": dp.collection_enabled()}) except Exception as e: - return web.json_response({"key": key, "rows": [], "error": str(e)}) + return web.json_response({"key": key, "rows": [], "enabled": False, + "error": str(e)}) + + @routes.post(UI_PREFIX + "/api/detail_probe") + async def _detail_toggle(req): + try: + body = await req.json() + except Exception: + body = {} + key = str(body.get("key") or "default").strip() or "default" + dp = _detail_module() + dp.set_collection_enabled(bool(body.get("enabled"))) + return web.json_response({"key": key, "rows": dp.rows(key), + "enabled": dp.collection_enabled()}) @routes.post(UI_PREFIX + "/api/detail_probe/clear") async def _detail_clear(req): @@ -2279,8 +2294,10 @@ if web is not None and PromptServer is not None: except Exception: body = {} key = str(body.get("key") or "default").strip() or "default" - _detail_module().clear_all(key) - return web.json_response({"key": key, "rows": []}) + dp = _detail_module() + dp.clear_all(key) + return web.json_response({"key": key, "rows": [], + "enabled": dp.collection_enabled()}) @routes.get(UI_PREFIX + "/api/temp") async def _temp_list(_req): diff --git a/tests/test_detail_probe.py b/tests/test_detail_probe.py index b582be38..562cbe59 100644 --- a/tests/test_detail_probe.py +++ b/tests/test_detail_probe.py @@ -398,3 +398,32 @@ def test_identical_conditioning_shifts_nothing(): row = dp.record("k", _edged(seed=0), settings=dict(cond)) assert row["cond_shift"] == 0.0 assert row["changed"] == {} + + +def test_the_detail_probe_switch_is_its_own(monkeypatch, tmp_path): + """Deliberately NOT shared with the block probe. They cost wildly different amounts — + this copies one latent per run, that gathers every block on every step — and answering + "is the measurement moving the picture?" requires running this one while that one is off. + A shared toggle made that experiment impossible.""" + import block_influence as bi + monkeypatch.delenv(dp._ENV_SWITCH, raising=False) + monkeypatch.delenv(bi._ENV_SWITCH, raising=False) + monkeypatch.setattr(dp, "_switch_path", lambda: str(tmp_path / "dp" / "enabled")) + monkeypatch.setattr(bi, "_switch_dir", lambda: str(tmp_path / "bi")) + + bi.set_collection_enabled(True) + assert dp.collection_enabled() is False # not dragged along by the other one + dp.set_collection_enabled(True) + bi.set_collection_enabled(False) + assert dp.collection_enabled() is True # and not switched off by it either + + +def test_the_detail_switch_persists_and_env_wins(monkeypatch, tmp_path): + monkeypatch.delenv(dp._ENV_SWITCH, raising=False) + monkeypatch.setattr(dp, "_switch_path", lambda: str(tmp_path / "dp" / "enabled")) + assert dp.collection_enabled() is False + dp.set_collection_enabled(True) + monkeypatch.delenv(dp._ENV_SWITCH, raising=False) + assert dp.collection_enabled() is True # from disk, after a restart + monkeypatch.setenv(dp._ENV_SWITCH, "0") + assert dp.collection_enabled() is False # env overrides