-
Notifications
You must be signed in to change notification settings - Fork 56
[Spyre-Next] Reworked forward call chain #872
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
Changes from 1 commit
caa563f
4b6b2e9
8fc82cc
a9ef99b
b167e6f
4593ddc
07729ab
7b497c8
01350cb
1304aaf
b5c2763
a147ae8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,11 @@ | |
|
|
||
| Architecture: | ||
| - OOT Registration: @RMSNorm.register_oot() replaces upstream at instantiation | ||
| - forward_oot(): Entry point for OOT dispatch, calls custom op for | ||
| torch.compile opacity | ||
| - Custom Op Boundary: torch.ops.vllm.spyre_rmsnorm is opaque to torch.compile, | ||
| so forward_native runs eagerly outside the compiled graph | ||
| - Separate Compilation: forward_static is compiled independently via maybe_compile | ||
| so _forward_spyre_impl runs eagerly outside the compiled graph | ||
| - Separate Compilation: forward_spyre is compiled independently via maybe_compile | ||
|
|
||
| Spyre Device Constraints: | ||
| - Minimum batch size: 64 (due to spyre constraint, automatically padded) | ||
|
|
@@ -19,8 +21,8 @@ | |
| - Algorithm: Transpose-based computation with torch.ops.spyre.full() | ||
|
|
||
| Limitations: | ||
| Currently the implementation in `_forward_vLLM_native` is similar to the | ||
| upstream implementation in `forward_static` from llm/model_executor/layers/layernorm.py, | ||
| Currently the implementation in `forward_spyre` is similar to the | ||
| upstream implementation in `forward_static` from vllm/model_executor/layers/layernorm.py, | ||
| but it DOES NOT use the promotion of the data types, as this is not | ||
| yet supported in torch-spyre. | ||
|
|
||
|
|
@@ -75,15 +77,15 @@ def __init__(self, *args, **kwargs): | |
| "expect numerical differences to upstream vLLM." | ||
| ) | ||
|
|
||
| def forward( | ||
| def forward_oot( | ||
| self, | ||
| x: torch.Tensor, | ||
| residual: torch.Tensor | None = None, | ||
| ) -> torch.Tensor | tuple[torch.Tensor, torch.Tensor]: | ||
| """Forward pass using custom op to bypass torch.compile. | ||
| """OOT forward pass using custom op to bypass torch.compile. | ||
|
|
||
| Delegates to torch.ops.vllm.spyre_rmsnorm which retrieves this layer | ||
| from forward_context.no_compile_layers and calls forward_impl outside | ||
| from the layer registry and calls _forward_spyre_impl outside | ||
| the compilation graph. This prevents torch.compile from inlining the | ||
| Spyre-specific operations. | ||
|
|
||
|
|
@@ -130,8 +132,6 @@ def forward_spyre( | |
| if x.shape[-1] != hidden_size: | ||
| raise ValueError(f"Expected hidden_size to be {hidden_size}, but found: {x.shape[-1]}") | ||
|
|
||
| x = x.transpose(-1, -2).contiguous() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried running rmsnorm repo tests on this branch it failed for me. I am not able to pinpoint why that might be. On main, they are passing, and apart from the function name changes, the only change I see is this.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @romitjain you need a very recent commit of torch-spyre for this to work. I reverted this rework from this PR and I'll move it into a separate PR.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bohnstingl So this change: 4b6b2e9 might not be needed. I was able to pass tests locally with and without this change. I have made a small PR here from my fork: bohnstingl#1. PR is for demonstration - feel free to merge/close and make changes directly here.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I merged it in. I will still leave the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rms_norm rework PR: #873 |
||
|
|
||
| variance_epsilon = torch.full( | ||
| x.shape, variance_epsilon, dtype=torch.float16, device=x.device | ||
| ) | ||
|
|
@@ -148,10 +148,9 @@ def forward_spyre( | |
| x_var = x[:, :, :variance_size_override] | ||
|
|
||
| # After transpose, hidden dim is now dim=0 | ||
| variance = x_var.pow(2).mean(dim=0, keepdim=True) | ||
| variance = x_var.pow(2).mean(dim=-1, keepdim=True) | ||
|
|
||
| x = x * torch.rsqrt(variance + variance_epsilon) | ||
| x = x.transpose(-1, -2).contiguous() | ||
|
|
||
| if weight is not None: | ||
| x = x * weight | ||
|
|
@@ -160,7 +159,7 @@ def forward_spyre( | |
| else: | ||
| return x, residual | ||
|
|
||
| def forward_native( | ||
| def _forward_spyre_impl( | ||
| self, | ||
| x: torch.Tensor, | ||
| residual: torch.Tensor | None = None, | ||
|
|
@@ -226,7 +225,7 @@ def _op_func( | |
| ) -> None: | ||
| """Custom op implementation — runs outside torch.compile graph.""" | ||
| layer = get_layer(layer_name) | ||
| result = layer.forward_native(x, residual) | ||
| result = layer._forward_spyre_impl(x, residual) | ||
|
|
||
| if residual is not None: | ||
| output_data, residual_data = result | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.