Repository navigation
Conversation
The in-graph cos/sin gather composite survives only one lookup per graph, so multi-position (M-RoPE) models (Qwen2/3-VL) cannot gather inside the graph. Add host-side gather_cos_sin_host / mrope_cos_sin so cos/sin are precomputed and fed as rope_cos / rope_sin graph inputs instead of position_ids. Workaround for apple#280 (runtime composite limitation).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #280.
The in-graph cos/sin gather composite survives only one lookup per graph (and only when
position_idsis the graph's own position input), so multi-position (M-RoPE) models — Qwen2/3-VL, whose time/height/width rows each need a lookup — can't gather inside the graph. Add host-sidegather_cos_sin_host/mrope_cos_sinthat do the table lookup with plain indexing, so models precompute cos/sin and feed them asrope_cos/rope_singraph inputs instead ofposition_ids. Runtime composite limitation (not a modeling bug); tracked via Feedback Assistant.Draft for preview.