Skip to content

Add native SDPA Perceiver implementation - #842

Open
alxmrs wants to merge 4 commits into
mainfrom
feature/native-sdpa-perceiver
Open

Add native SDPA Perceiver implementation#842
alxmrs wants to merge 4 commits into
mainfrom
feature/native-sdpa-perceiver

Conversation

@alxmrs

@alxmrs alxmrs commented Aug 14, 2026

Copy link
Copy Markdown
Member

Summary

  • replace perceiver-pytorch, flash-perceiver, and external flash-attn with native Perceiver and PerceiverIO components backed by torch.nn.functional.scaled_dot_product_attention
  • preserve existing config names: auto and sdpa use PyTorch dispatch, naive forces the math backend, and flash forces PyTorch FlashAttention
  • remove the retired packages from project metadata, lockfile, container setup/checks, quickstart, and installation/release documentation
  • exercise SDPA in the container smoke test and add a CUDA test that forces FlashAttention through forward and backward passes

Backward compatibility

The native modules retain the constructor shape and parameter hierarchy of perceiver-pytorch; its state dictionaries were verified to load directly and reproduce math-backend outputs within 1e-6. Existing model config values remain accepted without edits.

The separate flash-perceiver package used a different internal module layout, so checkpoints produced specifically by that backend may require migration even though their configs remain valid.

This PR does not include any research decoder/model variants or experiment/search configuration.

Container impact

No new CUDA binary is required. SDPA and its optimized kernels ship with PyTorch. The PhysicsNeMo image continues to supply the compatible PyTorch/CUDA stack, while its package verification and smoke test no longer require the removed external attention wheels.

Validation

  • focused Perceiver/encoder/decoder tests with all legacy packages physically absent — 25 passed, 1 CUDA test skipped locally
  • uv run --locked pytest -m "not manual and not cuda" -q — 394 passed, 2 skipped, 10 xfailed, 67 deselected
  • uvx pre-commit run --all-files — passed

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Comment thread src/samudra/config.py Outdated
Comment thread pyproject.toml
@alxmrs
alxmrs force-pushed the feature/native-sdpa-perceiver branch from 2169459 to 63483a9 Compare August 15, 2026 04:52
Comment thread scripts/build_quickstart_notebook.py Outdated
Comment thread notebooks/quickstart.ipynb Outdated
@alxmrs

alxmrs commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@codex may I have your review?

@alxmrs

alxmrs commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@fomo-bot may I have your review?

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f6623874b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/samudra/models/modules/perceiver.py
Comment thread docs/releasing.md
Comment on lines +24 to 26
# GPU — adds torchvision; PyTorch SDPA supplies optimized attention
uv add "samudra[cuda]"
pip install "samudra[cuda]"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove obsolete native-build installation steps

For users following the updated GPU installation section, the paragraph immediately below still says the cuda extra builds native kernels, requires a CUDA toolchain, and should be installed with --no-build-isolation. This commit removes the compiled attention dependencies and leaves the extra containing only torchvision, so those requirements and the pip command now contradict the package metadata and can unnecessarily complicate installation.

Useful? React with 👍 / 👎.

@fomo-bot

Copy link
Copy Markdown
Collaborator

Findings:

  • P3 docs/releasing.md:33: this paragraph is stale after the PR removes flash-attn/flash-perceiver and the [tool.uv] flash-attn build settings. It still says the cuda extra “builds native kernels,” needs a CUDA toolchain and preinstalled matching torch, and recommends pip install --no-build-isolation "samudra[cuda]". That now contradicts both the lines above it and pyproject.toml, where the cuda extra only adds torchvision. I’d update or delete lines 33-37 before merging so release/install guidance matches the new SDPA dependency story.

I checked the current head 3f662387. Existing review threads are resolved, the PR is MERGEABLE but still BLOCKED by required review, and the current check rollup is green.

Validation I ran locally:

  • git diff --check origin/main...HEAD
  • focused Perceiver/encoder/decoder/mini/quickstart tests: 29 passed
  • legacy perceiver-pytorch state dict compatibility probe for Perceiver and PerceiverIO: strict loads succeeded and sampled math-backend outputs matched within 1.2e-7
  • CUDA_VISIBLE_DEVICES= uv run --locked pytest -m "not manual and not cuda" -q: 394 passed, 2 skipped, 67 deselected, 10 xfailed
Debug info

@alxmrs

alxmrs commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@fomo-bot will you fix this P3 documentation issue?

@fomo-bot

Copy link
Copy Markdown
Collaborator

The P3 documentation issue is already fixed on the current PR head 6405b11b (Clarify SDPA installation and legacy decoding). docs/releasing.md no longer says the cuda extra builds native kernels or requires --no-build-isolation; it now states that Samudra does not compile attention extensions and relies on PyTorch SDPA’s available CUDA kernels.

I did not create a new commit because the branch was already synced with that fix and the worktree is clean. Validation on current head:

  • git diff --check origin/main...HEAD
  • uv run --locked pytest tests/test_perceiver.py tests/test_quickstart_config.py -q -> 13 passed
  • uvx pre-commit run --files docs/releasing.md tests/test_perceiver.py -> passed

PR state is still MERGEABLE; it remains blocked only by required review.

Debug info

@alxmrs

alxmrs commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@fomo-bot will you give this PR a final round of review? if you think it should be approved, then please approve it.

@fomo-bot fomo-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final review of current head 6405b11. I found no remaining blocking issues. The stale release-doc paragraph is fixed, and the decoder-query residual suggestion appears to be a false positive against the replaced perceiver-pytorch PerceiverIO behavior. Local validation passed: git diff --check origin/main...HEAD; focused Perceiver/encoder/decoder/mini/quickstart pytest slice; changed-file pre-commit.

@fomo-bot

Copy link
Copy Markdown
Collaborator

Approved PR #842 at current head 6405b11b.

Final review found no remaining blocking issues. I rechecked the two unresolved automated review threads: the release-doc issue is fixed in docs/releasing.md, and the decoder-query residual suggestion is not applicable because the replaced perceiver-pytorch PerceiverIO decoder path also did not add a queries residual.

Validation:

  • git diff --check origin/main...HEAD passed
  • uv run --locked pytest tests/test_perceiver.py tests/test_encoder.py tests/test_decoder.py tests/test_samudra_mini.py tests/test_quickstart_config.py -q -> 30 passed
  • uvx pre-commit run --files ... for changed files passed
  • Live PR state verified: APPROVED, MERGEABLE, CLEAN, with current CI checks green
Debug info

@alxmrs
alxmrs force-pushed the feature/native-sdpa-perceiver branch from 6405b11 to 416db16 Compare August 19, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants