sampling: check the values where they are used, not only where they are made
The NaN guard was on one of two doors. A FUNPACK_SETTINGS socket is typed by a string tag, so a dict reaches the load node without necessarily having passed through the settings node -- a raw API prompt does exactly that. Through that door, until_sigma = NaN made ALG a permanent no-op: sigma > nan is False at every step, so the anchor was never loosened, and the node reported "1 modifier applied: alg". Nothing about the run looked wrong, which is the only reason this is worth a commit of its own. Values are now checked against the declaration at the point they are consumed. The producer still checks too; that is not redundant, it is where a person editing JSON gets told immediately. Also recorded ALG's standing assumption rather than leaving it implicit: the anchor is latent frame 0. True of everything it can currently be wired into, untrue in general, and the denoise_mask already carries what would be needed to do it properly. Left undone because nothing yet pins elsewhere, and a derivation with no consumer cannot be checked against reality. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
134cee78b7
commit
fc977f4068
@@ -13,6 +13,15 @@ of its own. It attaches to the MODEL, so it applies to whatever sampler is wired
|
||||
H3" -- it needs a latent with a time axis and a pinned anchor, and any model with
|
||||
both gets it, including ones nobody here has heard of.
|
||||
|
||||
KNOWN ASSUMPTION: the anchor is latent frame 0. That is true of every i2v setup
|
||||
this can currently be wired into, and it is what v4 assumed. It is NOT true in
|
||||
general -- H3 can pin a keyframe at any frame -- so if an interior-pin mechanism
|
||||
is ever ported, ALG must take the frame indices from the pin rather than assume
|
||||
the first. The information to do it properly is already present in the
|
||||
denoise_mask, which is what marks the locked frames; deriving from it is left
|
||||
undone deliberately, because there is nothing yet that pins anywhere else and a
|
||||
derivation with no consumer cannot be checked against reality.
|
||||
|
||||
What is NOT ported from v4: the packed-layout path, which reached into a
|
||||
model-specific arrangement of the latent to find the video stream. Where the
|
||||
anchor is not a plain [B, C, T, H, W] tensor this stands down and says so once,
|
||||
|
||||
@@ -159,3 +159,12 @@ def test_the_proxy_is_transparent_for_everything_else():
|
||||
assert proxy.inner_model == "the real model"
|
||||
proxy.something = 7
|
||||
assert inner.something == 7, "an assignment did not reach the real denoiser"
|
||||
|
||||
|
||||
def test_a_threshold_that_is_not_a_number_would_disable_it_everywhere():
|
||||
"""Why the check at the load node matters, in ALG's own terms. NaN compares
|
||||
False against every sigma, so the anchor is never loosened at any step --
|
||||
and nothing about the run looks wrong."""
|
||||
from modules.sampling.alg.blur import use_blurred
|
||||
assert all(use_blurred(sigma, float("nan")) is False
|
||||
for sigma in (0.99, 0.5, 0.01))
|
||||
|
||||
@@ -163,8 +163,24 @@ class FunPackLoadModifiers(io.ComfyNode):
|
||||
if stripped:
|
||||
notes.append(f"replaced {stripped} modifier(s) already on this model")
|
||||
|
||||
# Checked HERE, not only where the payload was made. A FUNPACK_SETTINGS
|
||||
# socket is typed by a string tag, so a dict can reach this node without
|
||||
# ever passing through FunPackModifierSettings -- a raw API prompt, or a
|
||||
# second producer of that type. Validating only at the producer is a
|
||||
# guard on one of two doors, and the one left open let NaN through:
|
||||
# `sigma > nan` is False at every step, so a modifier became a permanent
|
||||
# no-op while reporting that it had been applied.
|
||||
problems = []
|
||||
chosen = {}
|
||||
for spec in ordered:
|
||||
values = (settings or {}).get(spec.id) or spec.defaults()
|
||||
values, said = schema_mod.check_values(spec, (settings or {}).get(spec.id))
|
||||
chosen[spec.id] = values
|
||||
problems.extend(said)
|
||||
if problems:
|
||||
raise RuntimeError("FunPack Load Modifiers:\n " + "\n ".join(problems))
|
||||
|
||||
for spec in ordered:
|
||||
values = chosen[spec.id]
|
||||
install = spec.provides[CAPABILITY]
|
||||
try:
|
||||
note = install(patched, values, key=KEY_PREFIX + spec.id)
|
||||
|
||||
@@ -252,3 +252,37 @@ def test_a_settings_payload_of_the_wrong_shape_is_refused_clearly(registry, patc
|
||||
|
||||
with pytest.raises(RuntimeError, match="keyed by module id"):
|
||||
FunPackLoadModifiers.execute(patcher, settings=["not", "a", "dict"])
|
||||
|
||||
|
||||
def test_values_are_checked_where_they_are_used_not_only_where_they_were_made(registry, patcher):
|
||||
"""The settings socket is typed by a string tag, so a dict can reach this
|
||||
node without passing through FunPackModifierSettings. Validating only at the
|
||||
producer guards one of two doors, and NaN walked through the other: sigma >
|
||||
nan is False at every step, so the modifier became a permanent no-op while
|
||||
reporting that it had been applied."""
|
||||
from modules.sampling.modifiers.nodes import FunPackLoadModifiers
|
||||
|
||||
registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={
|
||||
"until_sigma": {"type": "float", "default": 0.6, "min": 0.0, "max": 1.0, "label": "T"},
|
||||
})
|
||||
|
||||
with pytest.raises(RuntimeError, match="finite"):
|
||||
FunPackLoadModifiers.execute(patcher, settings={"alg": {"until_sigma": float("nan")}})
|
||||
|
||||
|
||||
def test_an_out_of_range_value_is_refused_at_this_node_too(registry, patcher):
|
||||
from modules.sampling.modifiers.nodes import FunPackLoadModifiers
|
||||
registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={
|
||||
"strength": {"type": "float", "default": 4.0, "min": 1.0, "max": 16.0, "label": "S"},
|
||||
})
|
||||
with pytest.raises(RuntimeError, match="above its maximum"):
|
||||
FunPackLoadModifiers.execute(patcher, settings={"alg": {"strength": 99.0}})
|
||||
|
||||
|
||||
def test_a_setting_no_module_declares_is_refused_at_this_node_too(registry, patcher):
|
||||
from modules.sampling.modifiers.nodes import FunPackLoadModifiers
|
||||
registry.specs["alg"] = _spec("alg", lambda t, v, key: "on", settings={
|
||||
"strength": {"type": "float", "default": 4.0, "min": 1.0, "max": 16.0, "label": "S"},
|
||||
})
|
||||
with pytest.raises(RuntimeError, match="no setting named"):
|
||||
FunPackLoadModifiers.execute(patcher, settings={"alg": {"strenght": 4.0}})
|
||||
|
||||
Reference in New Issue
Block a user