fix(attention): validate DIFFSYNTH_ATTENTION_IMPLEMENTATION before using it

[rakhimovv]

Resolves #12.

I asked in that issue which semantics you wanted and then realised I'd handed back a decision I could make: the strongest argument is repo-internal consistency, and get_attention_fn in openwam/model/action_backbone/components.py already answers every case. So this mirrors the sibling rather than inventing a policy — if you disagree with any case, it's one function to adjust.

What it does

initialize_attention_priority now matches WAM_ATTENTION_IMPL case for case:

override	before	after
flash_attention_3, FA3 not installed	selected, then NameError inside the kernel wrapper	warns, falls back to auto-detect
bogus	accepted, silently ran SDPA	ValueError listing the valid names
`` (empty) or whitespace	accepted, ran SDPA, logged a blank backend name	reads as no override → auto-detect
FLASH_ATTENTION_2	not normalized	.strip().lower(), honoured
unset	auto-detect	unchanged

The NameError is the one I'd call a plain bug, verified on Python 3.10.20, torch 2.7.1+cu128, flash_attn 2.8.3.post1, 4×H100:

$ DIFFSYNTH_ATTENTION_IMPLEMENTATION=flash_attention_3     # before
resolve_implementation -> flash_attention_3
attention_forward: NameError: name 'flash_attn_interface' is not defined

$ DIFFSYNTH_ATTENTION_IMPLEMENTATION=flash_attention_3     # after
WARN: DIFFSYNTH_ATTENTION_IMPLEMENTATION='flash_attention_3' requested but not available, falling back to auto-detect
resolved='flash_attention_2'  forward=(1, 2, 8, 16)

On the blank case, since it's the one that could bite

My first draft raised on an empty value, on the reasoning that it's reachable and meaningless. That was wrong, and I caught it before pushing: the sibling does os.environ.get(..., "").strip().lower() then if override:, so blank falls through to auto-detection there. Raising would have turned a stray export DIFFSYNTH_ATTENTION_IMPLEMENTATION= in someone's launch script into a hard failure at import — a real regression for no benefit. It now reads as no override, and test_blank_override_reads_as_no_override pins that for "" and " ".

That is also why I did not take @wayrise's torch_sdpa (unrecognized: "") suggestion from #11: with blank resolving to a real auto-detected backend, there is no blank name left for the report to render.

Tests

tests/test_attention_env_override.py, 8 cases, in its own file so it does not collide with #11's changes to test_attention_backend_dispatch.py. The availability flags are faked, so every case runs on CPU-only CI with none of the libraries installed — which is the configuration where this bug is invisible.

Against unpatched attention.py the shipped file fails 5/8; the 3 that pass either way are the already-correct behaviours, kept so a future change can't over-correct.

full suite      : 1976 passed, 20 skipped
ruff check      : clean
ruff format     : clean
compileall      : clean

Note on ordering

This branches from main, not from #11, and touches disjoint files (attention.py + a new test file, versus deploy/server.py + test_attention_backend_dispatch.py), so the two merge in either order without conflict.

[rakhimovv]

Force-pushed 2b8c8aa → 730e821; the first push had a red tests run and the failure was mine, not the change's.

While verifying that the new tests fail against unpatched sources I reverted attention.py with git checkout origin/main -- … and restored it with git checkout HEAD -- …. HEAD was the commit before I'd corrected the blank-value handling, so the restore silently reverted that correction and I amended the wrong content. CI then failed on exactly the two blank-value cases — the ones the correction exists for.

Worth naming because the check I used to confirm the restore (grep -c _available_implementations) matched both versions, so it certified nothing. grep -c 'if override:' discriminates, and I've re-verified against the commit rather than the working tree this time:

$ git show HEAD:openwam/model/video_backbone/wan/shared/core/attention/attention.py | grep -c 'if override:'
1

Locally: full suite 1976 passed / 20 skipped, ruff clean, and 5/8 of the new file fails against unpatched attention.py. No behaviour change from what the description says — the blank case reads as no override, as intended.

[wayrise]

Approving. Verified on an H200 (Python 3.10, torch 2.7.1+cu128), with FA3 and sage absent, which is the configuration the bug lives in.

check	result
tests/test_attention_env_override.py	8 passed
same file against origin/main's attention.py	5 failed, 3 passed
DIFFSYNTH_ATTENTION_IMPLEMENTATION=flash_attention_3	warns, resolves torch — no NameError
=bogus	ValueError, names the valid choices
= (blank)	resolves torch

You were right to reverse yourself on the blank case. I had suggested dressing it up in the logger back in #11 and you declined, and this is the better end of that: with blank resolving to a real backend there is no blank name left to render, so the diagnostics get fixed by not being touched. Raising on it would have turned a stray export DIFFSYNTH_ATTENTION_IMPLEMENTATION= into an import-time failure for no gain.

Leaving the auto-detection chain alone was also the right instinct — the fix is additive and the existing priority is visibly untouched, so nothing has to be proven about it.

I should say plainly that I duplicated this. I picked up #12 and wrote my own fix without checking for an open PR, and only found yours afterwards; I have closed #15 in favour of this one. Our two versions reached the same semantics independently, including the blank-value call, which is at least a decent sign the reading of components.py is the obvious one.

One non-blocking note

The only thing my version had that this does not is an invariant rather than a case: loop over every known implementation, assert each resolves to torch when no library is installed.

for override in ("flash_attention_3", "flash_attention_2", "sage_attention", "xformers", "torch"):
    assert _reinitialise(monkeypatch, shared, override) == "torch"

test_unavailable_override_warns_and_falls_back already covers the case that mattered, and this only adds value if a backend is added later without a matching test. Take it or leave it — not worth another round.