Skip to content

fix!: align embedding input order with PyTorch#808

Merged
voltjia merged 1 commit into
masterfrom
fix/align-embedding-python-api
Jul 24, 2026
Merged

fix!: align embedding input order with PyTorch#808
voltjia merged 1 commit into
masterfrom
fix/align-embedding-python-api

Conversation

@voltjia

@voltjia voltjia commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Restore the public Embedding input order to input, weight, matching torch.nn.functional.embedding.
  • Propagate the corrected order through the shared CUDA and Ascend implementations and the existing embedding tests.

Motivation

PR #799 accidentally changed the leading tensors from the public PyTorch order to the lower-level ATen order weight, indices. These are the most important positional arguments, and accepting them in reverse order makes the user-facing API misleading and incompatible with torch.nn.functional.embedding.

This is a follow-up to #799; no standalone issue is associated with the correction.

Type of Change

  • feat - new feature / new operator / new platform
  • fix - bug fix
  • perf - performance improvement (no behavioral change)
  • refactor - code restructuring without behavior change
  • test - adding or fixing tests only
  • docs - documentation only
  • build / ci - build system or CI configuration
  • chore - tooling, formatting, or other non-code changes
  • Breaking change

Platforms Affected

  • CPU (WITH_CPU)
  • NVIDIA (WITH_NVIDIA)
  • Iluvatar (WITH_ILUVATAR)
  • MetaX (WITH_METAX)
  • Cambricon (WITH_CAMBRICON)
  • Moore (WITH_MOORE)
  • Ascend (WITH_ASCEND)
  • PyTorch C++ bindings (WITH_TORCH)
  • Build system / CMake / CI
  • Python bindings / user-facing API

Smoke Test Result

Host: ssh nvidia
Image: accelerator-dev/nvidia:latest
PyTorch: 2.10.0a0+b4e4ee8

cmake ... -DINFINI_OPS_OPS=embedding
cmake --build build-nvidia --target ops -j 16
Build passed.

python -m pytest tests/test_embedding.py --devices cpu cuda -q
42 passed

Test Results on Supported Platforms

Platform Affected Build / Smoke Result Full Result / Notes
NVIDIA Yes embedding-target build passed tests/test_embedding.py: 42 passed
Iluvatar Yes Not run locally - no Iluvatar host available Uses the changed shared CUDA implementation
MetaX Yes Not run locally - no MetaX host available Uses the changed shared CUDA implementation
Cambricon No N/A - not affected N/A - not affected
Moore Yes Not run locally - no Moore host available Uses the changed shared CUDA implementation
Ascend Yes Not run locally - no Ascend host available Source order updated consistently
Full pytest output
Running 42 items in this shard
..........................................                               [100%]
42 passed

Benchmark / Performance Impact

N/A. This changes argument order and names only; the kernel implementation is unchanged.

Notes for Reviewers

API alignment

Interface Signature Alignment basis
InfiniOps primary interface embedding(input, weight, padding_idx, scale_grad_by_freq, sparse, out) PyTorch inputs in Python order; explicit out remains last per CONTRIBUTING.md.
PyTorch public API F.embedding(input, weight, padding_idx=None, max_norm=None, norm_type=2.0, scale_grad_by_freq=False, sparse=False) Pinned PyTorch source
  • The short overload is also restored to embedding(input, weight, out).
  • max_norm and norm_type remain out of scope because PyTorch handles renormalization in its Python functional wrapper before calling the low-level embedding operation.
  • The incorrect feat!: align non-InfiniLM operator interfaces #799 order cannot be retained as a deprecated overload: both orders have the same C++ type sequence, so parameter names cannot distinguish overload resolution. This PR restores the pre-feat!: align non-InfiniLM operator interfaces #799 public order instead of adding runtime dtype-based ambiguity.
  • clang-format 21.1.8 passed with --dry-run --Werror; Ruff passed for tests/test_embedding.py.

@voltjia
voltjia force-pushed the fix/align-embedding-python-api branch from e2589f8 to 089b33a Compare July 24, 2026 06:40
@voltjia
voltjia changed the base branch from master to fix/pin-ruff-version July 24, 2026 07:51
@voltjia
voltjia force-pushed the fix/align-embedding-python-api branch from 089b33a to 59566ad Compare July 24, 2026 08:02
Base automatically changed from fix/pin-ruff-version to master July 24, 2026 08:13
@voltjia
voltjia force-pushed the fix/align-embedding-python-api branch from 59566ad to 755077d Compare July 24, 2026 08:27
@voltjia
voltjia marked this pull request as ready for review July 24, 2026 08:36
@voltjia
voltjia requested a review from a team July 24, 2026 08:36
@voltjia voltjia changed the title fix!: align embedding input order with PyTorch fix!: align embedding input order with PyTorch Jul 24, 2026
@voltjia
voltjia merged commit 0079970 into master Jul 24, 2026
15 of 20 checks passed
@voltjia
voltjia deleted the fix/align-embedding-python-api branch July 24, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant