Skip to content

feat: prefer recipe ported einops - #1092

Open
drbh wants to merge 1 commit into
mainfrom
port-einops
Open

feat: prefer recipe ported einops#1092
drbh wants to merge 1 commit into
mainfrom
port-einops

Conversation

@drbh

@drbh drbh commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

this pr is an example of using the kernel porting tool to port the upstream einops into the kernel builder format

first we clone the upstream source*

# first clone the repo locally
git clone https://github.com/arogozhnikov/einops.git /tmp/einops-src

# next we pin to a specific commit (same as we s)
git -C /tmp/einops-src checkout ad2c8d67d08cbebb45e1aff2be21dc88635fc695

next we

# run the tool directly from the branch (could later be built into the cli tool)
nix run 'github:huggingface/kernels/pull/776/head#kernel-port' -- \
.ports/einops/port.kdl \ # the recipe to port
--dir /tmp/einops-src \ # the location of the source we cloned
--out ~/Projects/kernels-community/einops # the output location

running the command will show how each line of the recipe impacted the transformation and outputs all of the files added and deleted in the process.

[line  14] source              verified https://github.com/arogozhnikov/einops.git @ ad2c8d67d08c
[line  18] prune               removed 57 file(s), kept 27
[line  19] delete              removed 9 file(s)
[line  21] move                moved 18 file(s) to "torch-ext/einops"
[line  25] relativize_imports  rewrote 7 import(s) in 4 file(s)
[line  29] expect              0 occurrence(s), as expected
[line  36] replace             1 replacement(s) in 1 file(s)
[line  43] overlay             copied 11 file(s)
[line  45] expect              18 file(s), as expected
[line  48] expect              9 file(s), as expected
[line  52] manifest            wrote build.toml (noarch)
A build.toml
A flake.lock
A flake.nix
A tests/__init__.py
A tests/run_tests.py
A tests/test_einsum.py
A tests/test_examples.py
A tests/test_layers.py
A tests/test_ops.py
A tests/test_other.py
...
A torch-ext/einops/layers/torch.py
A torch-ext/einops/packing.py
A torch-ext/einops/parsing.py
A torch-ext/einops/py.typed
D .devcontainer/devcontainer.json
D .devcontainer/einops.Dockerfile
D .github/ISSUE_TEMPLATE/bug_report.md
D .github/ISSUE_TEMPLATE/extensions--introducing-new-operation---improve-notion.md
D .github/workflows/deploy_docs.yml
D .github/workflows/deploy_to_pypi.yml
D .github/workflows/run_tests.yml
D .github/workflows/test_notebooks.yml
D .gitignore
D .pre-commit-config.yaml
....
D scripts/setup.py
D scripts/test_notebooks.py

*the pinning step is optional and is just to make sure that the source matches the specific checkout that is at the top of the port.kdl file

source \
    repo="https://github.com/arogozhnikov/einops.git" \
    commit="ad2c8d67d08cbebb45e1aff2be21dc88635fc695"

important things

  • 9 of the 15 files in the overlay are tests we added that are not ported from upstream
  • the output is identical to the current manual port except for formatting of the build.toml, which is why the build.toml is the only change that is in the einops dir
  • the major benefit is that if the upstream library moves, we can update the commit in the recipe and rerun the command above

Signed-off-by: David Holtz <david.richard.holtz@gmail.com>
@github-actions github-actions Bot added the feature New functionality / capability label Aug 21, 2026
@drbh

drbh commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

for completeness, we can bump the commit hash from ad2c8d67d08cbebb45e1aff2be21dc88635fc695 to the latest head e0d5eb4fd535945ff65d309206d0f1754a926821

and trying to rerun the command shows

nix run 'github:huggingface/kernels/pull/776/head#kernel-port' -- .ports/einops/port.kdl --dir /tmp/einops-src --out einops
[line  14] source              verified https://github.com/arogozhnikov/einops.git @ e0d5eb4fd535
[line  18] prune               removed 56 file(s), kept 27
[line  19] delete              removed 10 file(s)
[line  21] move                moved 17 file(s) to "torch-ext/einops"
error: recipe line 25: relativize_imports: expected exactly 7 change(s) but made 13 - upstream drifted; review the new rewrites and update changes=

at this point I simply asked and llm to fix the recipe so the port works correctly and copy pasted the command and error output above.

the exact output of this process can be seen in #1093

@sayakpaul

Copy link
Copy Markdown
Member

I think we should have a single standard to start this. I would prefer if we required users to specify the source and the commit in the recipe itself instead of having them locally clone and checkout to a particular branch first. Or, that should be a better default behaviour, IMO.

9 of the 15 files in the overlay are tests we added that are not ported from upstream

We should port tests, too. Then it will be down on us to make modifications there e.g., for running specific tests on our CI, etc. Cc: @danieldk here as well.

@sayakpaul

Copy link
Copy Markdown
Member

Going to run the commands from the PR description and share my findings here as well.

@@ -0,0 +1,12 @@
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this automatically generated by huggingface/kernels#776?

Comment thread .ports/einops/port.kdl
find="from .packing import pack, unpack\n" \
with="from .packing import pack, unpack\nfrom . import array_api\n"

// The rewritten tests and the flake.

@sayakpaul sayakpaul Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not have to rewrite the tests much though. I think in the tests we would like to ensure that they use get_kernel() which already seems to be the case. So, I think we should rather provide similar instructions in the recipe to account for this.

pytest.mark.kernels_ci should probably be left to us because it's better if we decided which tests are lightweight and important enough to run on our CI.

Comment thread .ports/einops/port.kdl
@@ -0,0 +1,60 @@
recipe version=1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also decided to not maintain any top-level .ports?

*collect_test_backends(symbolic=True, layers=True),
]

# tensorflow.keras needs special way to compile,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't expect this term to appear in 2026.

Comment thread .ports/einops/port.kdl
@@ -0,0 +1,60 @@
recipe version=1

// einops

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we see an example where replace op is being used for torch op registration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New functionality / capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants