feat: detail probe gets its own switch, so it can run without the block probe
They were sharing one toggle on the argument that a second switch for the same decision is worse UI. That was wrong for a specific reason: the two probes cost wildly different amounts — the detail probe copies one latent per run, the block probe gathers ~17k x 5120 rows per block per step across 18 blocks — and allocator churn on that scale can change which GPU kernels get selected, so a probe can move the numbers it is measuring. Answering "is the measurement causing the variation?" needs one running while the other is off, which a shared toggle made impossible to perform. Also: the noise floor is now measured from your own repeat rows (median of 1 - structure across pairs where nothing changed) instead of graded against a fixed 0.98 that no real rig reaches. Other rows report their movement as a multiple of it, so "structure 0.42" reads as "6x your noise floor" — which is the part that means something.
This commit is contained in:
+38
-6
@@ -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):
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
+21
-4
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user