-
Notifications
You must be signed in to change notification settings - Fork 820
Thread tanh logit softcapping through FlashAttention (FA2 and FA3) #3391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nvegesna-netizen
wants to merge
37
commits into
NVIDIA:main
Choose a base branch
from
nvegesna-netizen:nvegesna/gemma2-softcap-core
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
69b4ffb
feat: thread tanh logit softcapping through FlashAttention (FA2, opt-…
nvegesna-netizen 9839ae0
fix: use raise instead of assert for the ONNX+softcap guard
nvegesna-netizen eb215b5
[pre-commit.ci] auto fixes from pre-commit.com hooks
nvegesna-netizen 2475521
refactor: move softcap reference into UnfusedDotProductAttention and …
nvegesna-netizen 3c5eb4a
fix(pytorch): gate FA3 softcap on existing NVTE_FLASH_ATTN_V3
nvegesna-netizen 900371a
fix(pytorch): disable FlashAttention 4 for softcap
nvegesna-netizen 5ecabac
fix(pytorch): disable FlashAttention 2 for softcap with dropout
nvegesna-netizen 818ce26
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 4cc2e9c
test: restore softcap dQ/dK/dV parity in the shared DPA harness
nvegesna-netizen 13d65a1
test: add softcap no-op and closed-form reference coverage
nvegesna-netizen 8be48f4
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 55a6ed9
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 8cb7f9c
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 9ba7803
fix(pytorch): align CP autograd backward arity with the softcap forwa…
nvegesna-netizen db6a119
test(pytorch): cover context parallelism with softcap
nvegesna-netizen 49ede92
fix(pytorch): apply softcap before the additive bias, matching FlashA…
nvegesna-netizen 18f67b7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] bce4d3f
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 4a3558a
Merge branch 'main' into nvegesna/gemma2-softcap-core
cyanguwa 8e49940
fix(test): skip the softcap no-op check when FusedAttention is unavai…
nvegesna-netizen 6387748
Merge remote-tracking branch 'origin/main' into nvegesna/gemma2-softc…
nvegesna-netizen 42975ac
fix(lint): silence missing-function-docstring on _IdentityWithMaskedG…
nvegesna-netizen d288f88
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 7349579
Merge remote-tracking branch 'origin/main' into nvegesna/gemma2-softc…
nvegesna-netizen 67277a0
feat: expose softcap on MultiheadAttention and TransformerLayer; trim…
nvegesna-netizen 33fe732
fix: append softcap to the MHA and TransformerLayer forward signatures
nvegesna-netizen a7b5160
test: consolidate the softcap tests
nvegesna-netizen 022a37b
test: assert TransformerLayer forwards softcap to both attention modules
nvegesna-netizen 2df3668
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 443f692
test: set explicit backend state in the softcap plumbing test
nvegesna-netizen 733144d
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen 146b2d5
Merge branch 'main' into nvegesna/gemma2-softcap-core
nvegesna-netizen cef71d8
chore(pytorch): tighten softcap comments and correct the FA4 attribution
nvegesna-netizen 8805255
fix(pytorch): probe softcap on all three FA3 entry points
nvegesna-netizen dbb36c3
test: exercise softcap at head_dim 128, not only 64
nvegesna-netizen 590c7ca
test: restore backend env vars in the softcap plumbing test
nvegesna-netizen f0cd672
fix(pytorch): reject a negative softcap instead of diverging silently
nvegesna-netizen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for thoroughly designing the tests, but I wonder if we could consolidate them a little bit, without losing meaningful test signals. How about this:
model_configs_softcap:softcap_1_0for the high softcap value and to testflash_attn_funcAPI;softcap_3_1(with "padding_causal" mask though) for a low softcap value and to testflash_attn_varlen_funcAPImodel_configs_softcap_referencefrom three configs to two, by merging GQA into the causal config; they are orthogonal to softcappingtest_dpa_softcap_vs_reference,test_dpa_softcap_bias_orderingandtest_dpa_softcap_qk_layer_scalinginto one test, parametrized over variants{"plain", "bias_outside_cap", "qk_layer_scaling"}; we can extract a helper function to do the repeated work: force a backend via env vars, build randn q/k/v, construct the module with the cap, compute a right and a wrong variant, assert the two differ by more than 10 * atol, assert TE matches the right one, return the DPA kwargs, forward kwargs and the two closuresThanks!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in a7b5160, following all four bullets.
model_configs_softcapis down tosoftcap_1_0andsoftcap_3_1, the latter switched topadding_causalas you suggested so it goes throughflash_attn_varlen_func.model_configs_softcap_referenceis down to two, with GQA merged into the causal config. The three tests are now onetest_dpa_softcap_vs_referenceparametrized over{plain, bias_outside_cap, qk_layer_scaling}, with a_softcap_variant_spechelper returning the DPA kwargs, the forward kwargs and a right and a wrong reference closure. The shared work of forcing the backend, building inputs, and asserting the two references differ by more than10 * atolnow happens once. The bias variant reuses_softcap_reference_attentionvia newbiasandcap_includes_biasarguments instead of duplicating its GQA and causal handling, which is what made the merged version shorter than the three it replaces. Causal is dropped from both special variants, as you asked.The matrix is 48 cases on a bf16 capable GPU, since
param_typespicks up bf16 when available, with 19 running and 29 skipping and each special variant running once on the non causal reference config.qk_layer_scalingis fp16 only. On an fp16 only machine it is 24, 10 and 14.Two things I checked before making the change, since consolidating tests can quietly remove the signal they exist for. Both variants still fail if their bug is reintroduced: capping the bias separates the two references by about 0.98, and dropping the
cap / layer_numberdivision by about 0.44, against the10 * atolthreshold, which is 0.20 in fp16 and 0.40 in bf16. Those margins hold because the survivingsoftcap_ref_1_0is the same shape the originals ran on. I also hoistedlayer_numberto_SOFTCAP_QK_LAYER_NUMBER = 8with a note that 3 is too small to separate, because that value is load bearing and would otherwise look arbitrary to the next person.On head dimensions, the two surviving configs cover 64 and 128:
dbb36c33movessoftcap_3_1to head_dim 128, since FA2 and FA3 compile a separate softcap kernel per head_dim and the input scale is head_dim invariant.I kept backward on the
plainvariant only, matching what runs today. Extending it to all three would be more coverage, but it is behaviour that has never run on hardware and I would rather not fold that in under a consolidation.