Add permute_1D_sparse_data Operation for Intel XPU - #72
Conversation
The test suite has been replaced with the patch |
33197a5 to
ea4b40f
Compare
Remove .gitkeep placeholders
- Add invert_permute kernel to CMake build - Implement invert_permute Python wrapper in ops.py - Register invert_permute operator with schema existence check - Add torch_library.h utility for schema validation
Add SYCL/XPU kernel implementation for invert_permute operation.
Add complete test coverage for invert_permute operator on XPU devices, covering correctness, validation, parity, and performance. Test coverage includes: - Correctness tests for int32/int64 with edge cases (empty, single element, identity, reverse, random permutations) - Input validation tests for invalid dimensions and dtypes - Meta function tests for torch.compile compatibility - PyTorch opcheck validation for operator conventions - Parametric tests with varying sizes (1 to 1M elements) - CPU-XPU parity tests to ensure consistent results - Performance benchmarks measuring execution time and bandwidth
Replace the custom standalone test_invert_permute.py with a git-am patch applied to upstream FBGEMM v1.7.0 misc_ops_test.py, following the torchcodec-xpu convention. The patch makes test_invert_permute run on XPU (permute.xpu(), gated on torch.xpu.is_available()) and skips the remaining operator tests that are not implemented on XPU.
Collapse the two type-specific kernel functors (InvertPermuteKernelInt32, InvertPermuteKernelInt64) into a single templated functor InvertPermuteKernel<index_t>, mirroring the reference CUDA kernel invert_permute_kernel<index_t>.
…cture - Fix test patches to match FBGEMM v1.8.0 tests. - Move test patches from test/patches/ subdirectory to patches/ at the package root level for better organization. Remove now-unnecessary .gitkeep file and update patch with correct base commit reference.
Replace TODO placeholders with actual commands to apply the FBGEMM XPU test patch and execute pytest for misc_ops_test.py in the CI pipeline.
e15792b to
a31b216
Compare
- CMakeLists: add permute_1d_sparse_data.cpp to build sources - ops.py: add Python wrapper with type hints - ops_registry.cpp: register operator schema in fbgemm namespace
Implement SYCL/XPU kernel implementation of permute_1D_sparse_data operator for sparse jagged/1D format data permutation.
Replaced test for upstream patched FBGEMM tests that enables testing XPU. This file is no longer needed.
a31b216 to
936e378
Compare
Signed-off-by: Felipe Leza Alvarez <felipe.leza.alvarez@intel.com>
| low=1, | ||
| high=L, | ||
| size=(T,), # 1D tensor with T elements | ||
| - device=torch.accelerator.current_accelerator(), |
There was a problem hiding this comment.
This should not be needed as torch.accelerator.current_accelerator() should return xpu as in trivial:
$ python -c "import torch; print(torch.xpu.is_available()); print(torch.accelerator.current_accelerator())"
True
xpu
There was a problem hiding this comment.
That line will be deleted by the patch and the device="xpu" will be added, the result code will be:
lengths = torch.randint(
low=1,
high=L,
size=(T,), # 1D tensor with T elements
device="xpu",
).type(index_dtype)
There was a problem hiding this comment.
I understand what this patch is currently doing and I am arguing that this should not be needed as upstream code should already do what you are trying to achieve - torch.accelerator.current_accelerator() should return xpu.
There was a problem hiding this comment.
Reverted, using the original torch.accelerator.current_accelerator() instead.
There was a problem hiding this comment.
Did you verify that it works?
There was a problem hiding this comment.
Yes, I did. The pytest command is already integrated into the Test fbgemm-xpu step: https://github.com/intel/torchlib-xpu/actions/runs/30579440544/job/90997602510#step:13:3
…st patch Signed-off-by: Felipe Leza Alvarez <felipe.leza.alvarez@intel.com>
| - @unittest.skipIf(*running_in_oss) | ||
| - @unittest.skipIf(*gpu_unavailable) | ||
| + @unittest.skipIf( | ||
| + gpu_unavailable[0] and not torch.xpu.is_available(), |
There was a problem hiding this comment.
Ok for now, but I suspect that going forward we will need to mimic gpu_unavailable.
| /** | ||
| * @brief Permute1DLengthsKernel operator implementation | ||
| */ | ||
| template <typename index_t> |
There was a problem hiding this comment.
Why we don't parameterize template for permute as in a reference: https://github.com/pytorch/FBGEMM/blob/3bc4f702d9e60c21d6b97150946e2e339625b73a/fbgemm_gpu/src/sparse_ops/sparse_permute_1d.cu#L16?
| * output[n] = sum(all input) | ||
| */ | ||
| template <typename T> | ||
| at::Tensor complete_cumsum_xpu(const at::Tensor& input) { |
There was a problem hiding this comment.
Same as above. This seems to be defined elsewhere in the cuda reference. Why do we define it here?
There was a problem hiding this comment.
@manuelhsantana why does exclusive_cumsum_xpu and complete_cumsum_xpu have the same code?
Also, the original code of permute_1D_sparse_data uses asynchronous_complete_cumsum that is an operator that we did implement to enable TorchRec, why wasn't that implementation used?
There was a problem hiding this comment.
complete_cumsum_xpu can be changed in PR #73 for asynchronouse_complete_cumsum if they provide the same results. But there is still the mystery of why exclusive_cumsum_xpu and complete_cumsum_xpu have the same code.
Introduce a permute_t template parameter on Permute1DLengthsKernel (defaulting to int32_t).
|
@aagalleg , this PR requires manual rebase for whatever reason (I am not sure what's wrong). Please, try to rebase. Once done I will attempt to merge. |
Depends on #71
Add
permute_1D_sparse_dataOperation for Intel XPUThis PR introduces the
permute_1D_sparse_dataoperator to fbgemm-xpu, enabling permutation of sparse data in jagged/1D format on Intel XPU devices.Changes
Core Implementation
permute_1d_sparse_data.cpp/h): SYCL implementation with three specialized functors for permuting lengths, indices, and weighted data, plus optimized cumsum helpers for offset computationops_registry.cpp): Registers the operator with PyTorch's dispatch system using conditional schema registration to avoid conflictsops.py): Clean Python wrapper function with type hints supporting optional weights and length sum parametersInfrastructure
CMakeLists.txt): Added SYCL kernel to build configurationTesting
test_permute_1d_sparse_data.py):cc: @manuelhsantana, @flezaalv