Conversation
There was a problem hiding this comment.
Pull request overview
Skips TypedStorage-related tests in the XPU test suite and documents why they’re not applicable, aligning test execution with the XPU skip lists.
Changes:
- Added
@unittest.skipannotations to TypedStorage-dependent tests intest_torch_xpu.py. - Expanded
skip_list_common.pywith a rationale for skipping TypedStorage tests on XPU. - Relaxed an XPU-specific
GradScalerstate-dict assertion to avoid relying on a typed XPU tensor class.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
test/xpu/test_torch_xpu.py |
Adds skips for TypedStorage tests and adjusts an XPU GradScaler assertion. |
test/xpu/skip_list_common.py |
Adds skip entries and rationale for TypedStorage-related tests on XPU. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TypedStorage is deprecated in PyTorch in favor of UntypedStorage paired with torch.dtype. XPU does not provide typed storage classes (e.g., torch.xpu.FloatStorage, torch.xpu.BFloat16Storage), so these tests cannot run on XPU. Changes: - Add @unittest.skip decorators to TypedStorage-related tests in test_torch_xpu.py to align with skip_list_common.py entries - Update skip_list_common.py with detailed explanation referencing torch/storage.py:L563 where the TypedStorage deprecation is documented Tests affected: - test_storage_setitem - test_storage_error - test_storage_error_no_attribute - test_typed_storage_deprecation_warning - test_typed_storage_internal_no_warning - test_tensor_storage_type - test_print
BartoszKokoszko
force-pushed
the
dev/bkokoszx/skip-TypedStorage-tests
branch
from
e2f0a80 to
cd4cff8
Compare
BartoszKokoszko
changed the title
guangyey
left a comment
guangyey
left a comment
There was a problem hiding this comment.
Thanks for your update!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @unittest.skipIf( | ||
| TEST_XPU, | ||
| "TypedStorage is deprecated and not available on XPU. See skip_list_common.py for details.", | ||
| ) | ||
| def test_typed_storage_internal_no_warning(self): |
There was a problem hiding this comment.
The new @unittest.skipIf(TEST_XPU, ...) skips this whole test whenever XPU is available, which also skips the CUDA coverage that this test includes via the torch.cuda.is_available() branch. If the goal is to skip only the XPU-typed-storage portion, consider guarding the XPU branch more narrowly (e.g., based on the presence of torch.xpu.FloatStorage) so CUDA behavior can still be exercised on systems that have both accelerators.
| @unittest.skipIf( | ||
| TEST_XPU, | ||
| "TypedStorage is deprecated and not available on XPU. See skip_list_common.py for details.", | ||
| ) | ||
| def test_storage_error_no_attribute(self): |
There was a problem hiding this comment.
With @unittest.skipIf(TEST_XPU, ...) added here, the if TEST_XPU: branch inside test_storage_error_no_attribute becomes unreachable. Consider removing the dead XPU branch (or replacing the decorator with a narrower capability/attribute check) to keep the test logic clear.
TypedStorage is deprecated in PyTorch in favor of UntypedStorage paired with torch.dtype. XPU does not provide typed storage classes (e.g., torch.xpu.FloatStorage, torch.xpu.BFloat16Storage), so these tests cannot run on XPU.
Changes:
Tests affected: