-
Notifications
You must be signed in to change notification settings - Fork 24.2k
Avoid calling fallback directly for symmetric memory tests #153520
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
base: gh/fegin/306/base
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/153520
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 1 New FailureAs of commit eff8f67 with merge base a13c8f2 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ag_output_vec = [] | ||
mm_outputs_vec = [] | ||
for context in test_contexts: | ||
with context(): |
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.
nit: it seems a little bit obscure to wrap the test in a loop (and use vector).
Since there is only 1 special context here, how about:
ag_output_0, mm_outputs_0 = torch.ops.symm_mem.fused_all_gather_matmul(
A_shard, Bs, gather_dim=gather_dim, group_name=group.group_name
)
with _test_mode():
ag_output_1, mm_outputs_1 = torch.ops.symm_mem.fused_all_gather_matmul(
A_shard, Bs, gather_dim=gather_dim, group_name=group.group_name
)
assert torch.allclose(ag_output_0, ag_output_1)
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.
(Like you did for the other test below)
Stack from ghstack (oldest at bottom):
Since we can just use _test_mode to dispatch the calls, we should use this method to also verify the function signature is consistent.
cc @H-Huang @awgu @wanchaol @fduwjj @wz337 @wconstab @d4l3k