Remove check_cpu_version and check_xpu_version helpers#4211
Remove check_cpu_version and check_xpu_version helpers#4211Xia-Weiwen merged 8 commits intopytorch:mainfrom
check_cpu_version and check_xpu_version helpers#4211Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4211
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c44ba5c with merge base 42ff8ed ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
check_cpu_version and check_xpu_version helpers
|
@claude review |
|
Claude finished @jerryzh168's task in 4m 9s —— View job Review of PR #4211: Remove
|
| if (w_int4x8.dtype == torch.uint8 or w_int4x8.shape[-1] > 1) and not ( | ||
| check_cpu_version(w_int4x8.device) | ||
| ): | ||
| if ( |
There was a problem hiding this comment.
this condition seems complicated, might be good to simplify if possible
There was a problem hiding this comment.
Oh, I am not familiar with the code here. And I feel the condition cannot be simplified. However, we can probably add a comment here to clarify the condition.
CC the last author @yanbing-j
I feel we should move this helper to |
Hi @jerryzh168 I feel that util function does not really simplify code. However, if you think the util function is better, I will use that. |
@jerryzh168 I have removed the |
|
@claude Please review again |
|
Claude finished @Xia-Weiwen's task in 1m 47s —— View job Re-review of PR #4211: Remove
|
| File | Status |
|---|---|
torchao/utils.py |
Clean removal of old helpers, clean addition of new ones |
torchao/dtypes/utils.py |
is_device removed, unused Union import cleaned up |
torchao/kernel/intmm.py |
Correct replacement |
torchao/quantization/utils.py |
Correct replacements. The condition at line 483-485 is still complex but this PR doesn't change its logic, just the device-check syntax |
torchao/quantization/linear_quant_modules.py |
Correct. Line 119 uses device.type == "cpu" directly since device is not a tensor — this is appropriate |
torchao/quantization/qat/linear.py |
Correct replacement |
torchao/prototype/hqq/hqq_tinygemm_linear.py |
Correct. Previously mixed is_device and check_cpu_version; now uniformly uses is_on_device |
torchao/prototype/tensor_conversion/api.py |
Correct replacement, removed stale # TODO: move the function to torchao.utils comment |
test/dtypes/test_affine_quantized.py |
Dead code removed (all branches were pass). Note: do_int4 param is now unused — could be cleaned up in a follow-up |
test/quantization/test_quant_primitives.py |
Correct replacements |
CI: Ruff linting and PR label check pass. GPU/regression tests still running.
Summary: LGTM. The changes are correct and the new helpers are an improvement in readability and consistency. The only follow-up suggestions (not blockers) are adding the new functions to __all__ and cleaning up the unused do_int4 parameter in tests.
--- |
There was a problem hiding this comment.
Pull request overview
This PR removes the check_cpu_version / check_xpu_version helpers (and related call sites) and consolidates device checks into new is_on_device / not_on_device utilities.
Changes:
- Delete
check_cpu_versionandcheck_xpu_versionfromtorchao/utils.pyand introduceis_on_device/not_on_device. - Replace device/version helper usage across quantization, kernel, and prototype code with the new device-check utilities.
- Remove the now-unused
is_devicehelper fromtorchao/dtypes/utils.pyand update tests accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
torchao/utils.py |
Removes CPU/XPU version-check helpers; adds shared tensor device predicates. |
torchao/quantization/utils.py |
Updates int4 packing/unpacking conditionals to use is_on_device / not_on_device. |
torchao/quantization/qat/linear.py |
Replaces is_device(...) usage with is_on_device(...). |
torchao/quantization/linear_quant_modules.py |
Replaces is_device(...) usage with is_on_device(...) and direct device.type checks. |
torchao/prototype/tensor_conversion/api.py |
Switches CUDA device checks to is_on_device. |
torchao/prototype/hqq/hqq_tinygemm_linear.py |
Removes CPU version helper usage in favor of is_on_device. |
torchao/kernel/intmm.py |
Replaces CPU-version helper usage with is_on_device. |
torchao/dtypes/utils.py |
Removes the unused is_device helper. |
test/quantization/test_quant_primitives.py |
Updates test logic to use is_on_device / not_on_device. |
test/dtypes/test_affine_quantized.py |
Removes now-unused imports and dead conditional block related to CPU/XPU version helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
the util is to make sure it works for multiple situations, like device being string, being torch.device type etc., it's more for robustness of the check I don't understand the motivation of |
| if (not (check_cpu_version(int_data.device))) and ( | ||
| not (check_xpu_version(int_data.device)) | ||
| ): | ||
| if not_on_device(int_data, ["cpu", "xpu"]): |
There was a problem hiding this comment.
I see, I don't think we need special util just for having a list of args, I think it's better to just use is_device
The motivation is to simplify code and make it more straightforward: the new utils work on tensors so that callers don't have to write Anyway, I will rollback to |
|
@claude review |
|
Claude finished @Xia-Weiwen's task in 1m 53s —— View job Review of PR #4211: Remove
|
| File | Status |
|---|---|
torchao/utils.py |
Clean removal of check_cpu_version and check_xpu_version (12 lines deleted). No other changes needed — _is_device already exists at line 48. |
torchao/kernel/intmm.py |
Correct: check_cpu_version(scales1.device) → _is_device("cpu", scales1.device). Import updated. |
torchao/prototype/hqq/hqq_tinygemm_linear.py |
Correct: Two check_cpu_version calls and one _is_device import consolidated. Now imports only _is_device from torchao.utils. |
torchao/quantization/utils.py |
Correct: All check_cpu_version/check_xpu_version calls replaced with _is_device. The complex condition at lines 462-467 preserves the original logic exactly. |
test/quantization/test_quant_primitives.py |
Correct: Same pattern of replacements. Imports updated. |
No remaining references to check_cpu_version or check_xpu_version exist anywhere in the codebase.
One observation (not a blocker): The argument order of _is_device is inconsistent across the codebase. This PR correctly uses _is_device("cpu", device) matching the function signature _is_device(target_device_str, device). However, some pre-existing callers in torchao/quantization/linear_quant_modules.py and torchao/quantization/qat/linear.py (not changed by this PR) use the reversed order: _is_device(q_weight.device.type, "cpu"). This happens to work because string equality is symmetric, but it's confusing. Worth a follow-up cleanup to standardize.
Summary: LGTM. The changes are correct, minimal, and CI looks good.
jerryzh168
left a comment
There was a problem hiding this comment.
yeah I think this is cleaner, please update the summary as well
This PR removes
check_cpu/xpu_versionsince they are ambiguous and changes all uses of these helpers to_is_device("cpu/xpu", tensor.device).