use stock pytorch defintion of python_ref_db for test_ops #2303
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the handling of python_ref_db in XPU test utilities to use PyTorch's stock definition instead of maintaining a custom copy. The changes eliminate redundant storage and manipulation of python_ref_db while filtering it locally where needed.
Key Changes:
- Removed custom storage of
python_ref_dbin the XPU test wrapper class - Simplified
align_supported_dtypeslogic by removing complex conditional checks - Added skip list entries for newly exposed test cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/xpu/xpu_test_utils.py | Removed custom python_ref_db storage, simplified dtype alignment logic, and created local filtered version where needed |
| test/xpu/skip_list_common.py | Added skip entries for _vdot_, _dot_, _flash_attention_, and _efficient_attention_ operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) or opinfo.name in _ops_without_cuda_support: | ||
| if opinfo.name in _ops_without_cuda_support: | ||
| opinfo.dtypesIf["xpu"] = opinfo.dtypes | ||
| else: |
There was a problem hiding this comment.
The simplified logic in align_supported_dtypes no longer checks if opinfo.name is in _xpu_computation_op_list before setting dtypes. This means operations not in _xpu_computation_op_list but also not in _ops_without_cuda_support will now take the else branch and modify backward_dtypes, which may not be the intended behavior. The original condition ensured only operations in the computation list were processed this way.
| else: | |
| elif opinfo.name in _xpu_computation_op_list: |
|
|
||
| import torch | ||
| from torch import bfloat16, cuda | ||
| from torch import cuda |
There was a problem hiding this comment.
The removal of bfloat16 from the import statement leaves it unused elsewhere in the code, but line 927 previously referenced bfloat16 when adding it to backward_dtypes. Verify that removing the bfloat16 handling doesn't break backward compatibility for operations that require it.
Performance outliers, please check!
|
| "_vdot_", | ||
| "_dot_", | ||
| "_flash_attention_", | ||
| "_efficient_attention_", |
There was a problem hiding this comment.
do we need to link issue for those cases?
There was a problem hiding this comment.
They are unsupported ops, do you know whether we have plan on dot and vdot?
disable_build
disable_e2e
disable_distributed