Repository navigation
feat: prefer recipe ported einops #1092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b626f63
47ba186
4c01d31
324a1e2
dbad83e
7193964
48c52d0
4a6e1bf
cb337de
11d283c
ac0b226
61add17
7b7fc4d
9c378a0
8707a62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Ported kernels are generated: <kernel>/port/port.kdl plus <kernel>/port/overlay | ||
| # are the inputs, and <kernel>/src is kernel-port's output, committed so that | ||
| # what ships stays reviewable and auditable. | ||
| # | ||
| # Collapse the generated tree in diffs so review lands on the inputs. The output | ||
| # is not taken on trust: .github/workflows/check-ports.yaml regenerates every | ||
| # port and fails if <kernel>/src does not match byte for byte. | ||
| */src/** linguist-generated=true |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| name: Check Ports | ||
|
|
||
| # Verify committed src trees match their port recipes. PRs check changed ports; | ||
| # checker changes, pushes to main, scheduled runs and manual runs check all ports. | ||
| on: | ||
| push: | ||
| branches: [main] | ||
| # Catch kernel-port regressions even when no port changes. | ||
| schedule: | ||
| - cron: "0 6 * * *" | ||
| pull_request: | ||
| branches: [main] | ||
| types: [opened, synchronize, reopened] | ||
| paths: | ||
| - "*/port/**" | ||
| - "*/src/**" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So for kernels containing valid porting recipes, we expect their sources to be present under |
||
| - "scripts/check_ports.py" | ||
| - ".github/workflows/check-ports.yaml" | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| check-ports: | ||
| name: Check ports reproduce | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 | ||
|
|
||
| - name: Regenerate selected ports and compare | ||
| env: | ||
| # Refresh the runner to catch upstream changes. | ||
| # TODO: switch back to main once huggingface/kernels#877 is merged. | ||
| KERNEL_PORT: "nix run --refresh github:huggingface/kernels/kernel-port-no-importlib#kernel-port --" | ||
| BASE_REF: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || '' }} | ||
| run: | | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the overhead of running this is high, we could consider running it only for the kernel being added / changed at least for PRs?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this already uses --changed-since for prs, so only changed ports are checked. changes to the checker itself run all ports. renamed the step since "every port" was misleading |
||
| if [ -n "$BASE_REF" ]; then | ||
| python3 scripts/check_ports.py --changed-since "$BASE_REF" | ||
| else | ||
| python3 scripts/check_ports.py | ||
| fi | ||
|
sayakpaul marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ build | |
| # Editor files | ||
| .* | ||
| !.github/ | ||
| !/.gitattributes | ||
|
|
||
| # Python stuff | ||
| __pycache__/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| import os | ||
|
|
||
| # Use the CI backends by default, while allowing local overrides. | ||
| os.environ.setdefault("EINOPS_TEST_BACKENDS", "torch,numpy") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| recipe version=1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay so IIUC we didn't need any overlay in this case? I thought the flake.nix would go to the overlay directory? How are we handling that?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea no overlay is needed for einops since everything comes from upstream.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oh okay. Earlier, I think we wanted to keep the external files in the overlay. What is the general guidelines behind putting stuff under an |
||
|
|
||
| // Pin the exact clean upstream tree this recipe transforms. | ||
| source \ | ||
| repo="https://github.com/arogozhnikov/einops.git" \ | ||
| commit="ad2c8d67d08cbebb45e1aff2be21dc88635fc695" | ||
|
|
||
| // Retain only the Python package and its upstream tests. | ||
| prune keep="einops/**" | ||
|
|
||
| // Lift upstream tests out of the package into the kernel test root. | ||
| move \ | ||
| from="einops/tests" \ | ||
| to="tests" | ||
|
|
||
| // Place the distributable package in the torch extension tree. | ||
| move \ | ||
| from="einops" \ | ||
| to="torch-ext/einops" | ||
|
|
||
| // Make package-internal imports work under the Hub-generated module name. | ||
| relativize_imports \ | ||
| in="torch-ext/einops/**" \ | ||
| package_root="torch-ext/einops" | ||
|
|
||
| // Expose array_api because Hub consumers cannot import it by package name. | ||
| ensure_import \ | ||
| in="torch-ext/einops/__init__.py" \ | ||
| from="." \ | ||
| name="array_api" | ||
|
|
||
| // Point imports of the former einops.tests package at the lifted tests. | ||
| remap_module \ | ||
| in="tests/{__init__,test_*}.py" \ | ||
| from="einops.tests" \ | ||
| to="tests" | ||
|
|
||
| // Make imports between lifted test modules package-relative. | ||
| relativize_imports \ | ||
| in="tests/{__init__,test_*}.py" \ | ||
| package_root="tests" | ||
|
|
||
| // Resolve product imports through the locally built Hub kernel. The package | ||
| // init binds it privately so `from . import *` cannot pick it up. | ||
| kernelize_imports \ | ||
| in="tests/__init__.py" \ | ||
| package="einops" \ | ||
| binding="_einops" \ | ||
| kernel="kernels-community/einops" \ | ||
| version=1 | ||
|
|
||
| kernelize_imports \ | ||
| in="tests/test_*.py" \ | ||
| package="einops" \ | ||
| kernel="kernels-community/einops" \ | ||
| version=1 | ||
|
|
||
| // Default to Torch and NumPy tests, with an EINOPS_TEST_BACKENDS override. | ||
| overlay from="overlay" | ||
|
|
||
| // Use the supported replacement for NumPy 2's removed in1d API. | ||
| replace in="tests/test_layers.py" \ | ||
| find="numpy.in1d(grad, [0, 1])" \ | ||
| with="numpy.isin(grad, [0, 1])" | ||
|
|
||
| // Select every upstream test for the kernel CI suite. | ||
| mark_tests in="tests/test_*.py" marker="kernels_ci" changes=63 | ||
|
|
||
| // Emit the noarch kernel manifest and supported runtime backends. | ||
| manifest \ | ||
| name="einops" \ | ||
| version=1 \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More for the future, but it would be nice to set a variable
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah nice idea! will keep this in mind with when next updating the porting tool. thanks |
||
| edition=5 \ | ||
| license="MIT" \ | ||
| upstream="https://github.com/arogozhnikov/einops.git" \ | ||
| backends="cpu,cuda,metal,rocm,xpu" \ | ||
| repo_id="kernels-community/einops" \ | ||
| noarch=#true | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are some of these tests carrying the "Test in CI" marker?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, the recipe marks all 63 test functions with kernels_ci. the last ci run's gpu test step passed in 12 seconds, so kept the full selection |
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.