Skip to content

Issue 381 Fix: Add attention mask to SAM3 export and runtime - #385

Merged
kevchengcodes merged 3 commits into
apple:mainfrom
kevchengcodes:issue381-fix
Oct 29, 2026
Merged

kevchengcodes merged 3 commits into
apple:mainfrom
kevchengcodes:issue381-fix

Conversation

@kevchengcodes

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix
  • Small feature (agreed in an issue)
  • New model (export recipe only)
  • New model (export recipe and new runtime support)
  • Documentation update

Motivation and Context

Issue #381 found that SAM3 Lite was not passing an attention mask to the detector, which led to inaccurate scores when compared against the HF reference. This is because without the attention mask, the detector was cross-attending to all the padding slots and averaging them into the scoring vector alongside the real prompt. So short prompts were getting drowned out by the padding.

While investigating, I also found that SAM3 Full's example_inputs in Torch was causing token truncation. Before, we had:

text_inputs = processor.tokenizer(["dummy"], return_tensors="pt")

Which would truncate any prompt to 3 tokens (i.e. max of 2 word prompts). This PR changes it to:

    text_inputs = processor.tokenizer(
        ["dummy"],
        padding="max_length",
        max_length=config.max_text_seq_len,
        return_tensors="pt",
    )

So that it properly allows prompts up to the max sequence length (32). Additionally, we add the attention mask to SAM3 Full as well.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

I reproduced the error using the script provided in the issue, and verified that these fixes address the problem.

@kevchengcodes kevchengcodes self-assigned this Oct 27, 2026
@kevchengcodes
kevchengcodes requested a review from a team as a code owner October 27, 2026 21:48
Comment thread swift/Sources/CoreAIImageSegmenter/ImageSegmentationEngine.swift

@carinapeng carinapeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix!

@stikves stikves left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes existing 0.2 runtimes incompatible with the new exports.

We should bump the metadata version number to 0.3, just like the recent batched LLM exports, and make runtime compatible with both (if needed), or explicitly reject 0.2

@kevchengcodes

kevchengcodes commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

This makes existing 0.2 runtimes incompatible with the new exports.

We should bump the metadata version number to 0.3, just like the recent batched LLM exports, and make runtime compatible with both (if needed), or explicitly reject 0.2

@stikves I tested an old export with the new runtime and a new export with the old runtime. Both cases work, so I think we're okay keeping the same version.

@kevchengcodes
kevchengcodes merged commit 0aff938 into apple:main Oct 29, 2026
3 checks passed
@kevchengcodes
kevchengcodes deleted the issue381-fix branch October 29, 2026 02:38

@spacexm406-web spacexm406-web 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.

workflows/repo-status

Constant improvement and performance daily expansion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants