DIFFSYNTH_ATTENTION_IMPLEMENTATION is unvalidated: naming an uninstalled backend raises NameError at dispatch

[rakhimovv]

DIFFSYNTH_ATTENTION_IMPLEMENTATION is taken unvalidated, so naming an uninstalled backend crashes at dispatch — while its sibling WAM_ATTENTION_IMPL validates and falls back. This is the root cause behind the diagnostics fixes in #9 and #11; those made the report honest, but the underlying selection is still unchecked.

The asymmetry

get_attention_fn in openwam/model/action_backbone/components.py does both checks:

override = os.environ.get("WAM_ATTENTION_IMPL", "").strip().lower()
if override:
    if override not in _BACKEND_MAP:
        raise ValueError(f"Unknown WAM_ATTENTION_IMPL='{override}'. Choose from: {list(_BACKEND_MAP.keys())}")
    fn = _BACKEND_MAP[override]()
    if fn is None:
        logger.warning("WAM_ATTENTION_IMPL='%s' requested but not available, falling back to auto-detect", override)

initialize_attention_priority in openwam/model/video_backbone/wan/shared/core/attention/attention.py does neither — it lowercases and returns:

if os.environ.get("DIFFSYNTH_ATTENTION_IMPLEMENTATION") is not None:
    return os.environ.get("DIFFSYNTH_ATTENTION_IMPLEMENTATION").lower()

Three failure modes

All three run on Python 3.10.20, torch 2.7.1+cu128, flash_attn 2.8.3.post1, 4×H100, on main at c5b6b81 (which includes #11). FA3 and sage are not installed here, which is the ordinary case.

1. Naming an unavailable backend crashes at dispatch. The device/dtype guard from #9 admits CUDA+half, but nothing checks the library actually imported:

$ DIFFSYNTH_ATTENTION_IMPLEMENTATION=flash_attention_3
ATTENTION_IMPLEMENTATION = 'flash_attention_3'
FLASH_ATTN_3_AVAILABLE   = False
resolve_implementation   -> flash_attention_3
attention_forward: NameError: name 'flash_attn_interface' is not defined

This is the one I'd call a plain bug. WAM_ATTENTION_IMPL=flash3 on the same machine warns and falls back; this raises a NameError from inside the kernel wrapper.

2. An unrecognized value is accepted silently.

$ DIFFSYNTH_ATTENTION_IMPLEMENTATION=bogus
ATTENTION_IMPLEMENTATION = 'bogus'   -> attention_forward runs fine (plain SDPA)

attention_forward's else branch is doing the right thing, but a typo'd backend name is indistinguishable from a working one — the user asked for a fused kernel and silently got SDPA.

3. The empty string is reachable and behaves the same, because the lookup guards on is not None rather than truthiness:

$ DIFFSYNTH_ATTENTION_IMPLEMENTATION=
ATTENTION_IMPLEMENTATION = ''   -> resolve_implementation -> ''   -> SDPA

This is also why the deploy report currently logs a blank backend name for that case — @wayrise raised the cosmetic in #11 and I deliberately left it alone there, because annotating the symptom in the logger seemed worse than fixing the cause here.

Suggested fix

Mirror the sibling. In initialize_attention_priority, after reading the override: .strip().lower(), reject a name outside the known set, and check the corresponding *_AVAILABLE flag before returning it, falling back to auto-detection with a warning when the library is missing.

That closes all three modes and makes the two env vars behave the same way, which seems worth having on its own — right now WAM_ATTENTION_IMPL=nonsense raises immediately and DIFFSYNTH_ATTENTION_IMPLEMENTATION=nonsense runs a whole training job.

One genuine design question I did not want to decide for you: unrecognized value — raise or warn-and-fall-back? components.py raises for an unknown name but only warns for an unavailable one, and I can see the argument either way for an env var inherited from DiffSynth-Studio, where a stricter reading might break someone's existing invocation. Availability-checking (mode 1) seems unambiguous to me; the membership check is the part that's your call.

Happy to send the PR once you've picked the semantics — it's a contained change to one function plus tests, and I have the box to verify it on. Or say the word and I'll leave it with you.

[rakhimovv]

Sent as #14, implementing the sibling's semantics exactly rather than waiting on your answer — on reflection the question I asked was one I could settle: repo-internal consistency with WAM_ATTENTION_IMPL is a stronger argument than my worry about diverging from DiffSynth-Studio, and get_attention_fn already has a considered answer for every case. Easy to adjust if you'd have picked differently.

One correction to this issue while I'm here: I listed the empty string as a failure mode, and the fix does not treat it as one. The sibling reads a blank value as "no override" and falls through to auto-detection, so raising on it would turn a stray export DIFFSYNTH_ATTENTION_IMPLEMENTATION= into a hard failure at import for no benefit. My first draft did raise; I caught it before pushing. Blank now resolves to a real auto-detected backend, which also disposes of the blank-name log line without touching the logger.

[wayrise]

Confirmed all three modes on an H200 box before touching anything — including the NameError, which is as you describe: resolve_implementation admits flash_attention_3 on CUDA+half because the #9 guard only checks device and dtype, and flash_attention_3() then dereferences a name the try/except import never bound.

Thanks for filing it rather than papering over the blank name in the logger. That was the right call, and it's visible in the fix: with the cause addressed, the deploy report reads Wan shared core : torch for a blank override with nothing added to _qualify_backend.

On your design question — raise. Matching components.py, so the two variables finally behave the same way:

	unknown name	known name, library missing
WAM_ATTENTION_IMPL	ValueError	warn, fall back
DIFFSYNTH_ATTENTION_IMPLEMENTATION (now)	ValueError	warn, fall back

I weighed the DiffSynth-Studio compatibility argument you raised and came down against it: only five values were ever meaningful here, so anyone spelling one correctly is unaffected, and anyone who is not is currently training on SDPA without knowing. That is the case worth breaking loudly.

One consequence I should flag since it goes slightly beyond what you proposed: because ATTENTION_IMPLEMENTATION is resolved at module scope, a rejected name now raises at import rather than at first dispatch. Deliberate, and _log_attention_backends already catches it per subsystem, but it is a real behaviour change and I would rather you saw it stated than found it.

Rather than leave you waiting on a decision, I went ahead and wrote it: #15. Auto-detection order is unchanged and now pinned by a test. Six cases, all CPU-only, 4 of which fail against origin/main; full suite 1979 passed / 20 skipped with and without flash-attn.

Review very welcome — you found this one and know the corners better than the patch does. If you would rather have written it yourself, say so and I will close mine; no attachment to the authorship.