Skip to content
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

[Keras Ops] Add einops-style rearrange() to keras.ops #20733

Merged
merged 11 commits into from
Jan 15, 2025

Conversation

DavidLandup0
Copy link
Contributor

Context

Closes #20332.

Add an einops style rearrange() operation to the keras.src.ops.einops.
Making a new module for this makes sense if we plan to add other similar operations and want to re-use some of the private-marked utility methods here or generalize the logic further.

It makes use of solely reshape() and transpose(), so these natively run on all backends. Furthermore, only these two operations are applied so it should be fine with symbolic tensors (sometimes, einops doesn't work correctly with symbolic tensors in a computation graph).

Comparison with einops

Direct comparison with einops, using the examples from the official documentation:

import numpy as np
import keras
import einops

tensor = np.random.randn(2, 10, 10, 3)

examples = [
    (tensor, 'b h w c -> b h w c', {}),
    (tensor, 'b h w c -> b c h w', {}),
    (tensor, 'b h w c -> (b h) w c', {}),
    (tensor, 'b h w c -> h (b w) c', {}),
    (tensor, 'b h w c -> b (c h w)', {}),
    (tensor, 'b (h h1) (w w1) c -> b (h w1) (w h1) c', {'h1': 2, 'w1': 2}),
    (tensor, 'b (h h1) (w w1) c -> b c (h w1) (w h1)', {'h1': 2, 'w1': 2}),
    (tensor, 'b (h1 h) (w1 w) c -> (b h1 w1) h w c', {'h1': 2, 'w1': 2}),
    (tensor, 'b (h h1) (w w1) c -> b h w (c h1 w1)', {'h1': 2, 'w1': 2})
]

results = []

for data, pattern, axes_lengths in examples:
    keras_result = keras.ops.rearrange(data, pattern, **axes_lengths)
    einops_result = einops.rearrange(data, pattern, **axes_lengths)
    
    are_equal = np.allclose(keras_result, einops_result)
    
    result_entry = {
        "pattern": pattern,
        "from": data.shape,
        "to": keras_result.shape,
        "equal": are_equal
    }
    
    results.append(result_entry)

Results in:

{'pattern': 'b h w c -> b h w c', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 10, 10, 3]), 'equal': True}
{'pattern': 'b h w c -> b c h w', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 3, 10, 10]), 'equal': True}
{'pattern': 'b h w c -> (b h) w c', 'from': (2, 10, 10, 3), 'to': TensorShape([20, 10, 3]), 'equal': True}
{'pattern': 'b h w c -> h (b w) c', 'from': (2, 10, 10, 3), 'to': TensorShape([10, 20, 3]), 'equal': True}
{'pattern': 'b h w c -> b (c h w)', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 300]), 'equal': True}
{'pattern': 'b (h h1) (w w1) c -> b (h w1) (w h1) c', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 10, 10, 3]), 'equal': True}
{'pattern': 'b (h h1) (w w1) c -> b c (h w1) (w h1)', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 3, 10, 10]), 'equal': True}
{'pattern': 'b (h1 h) (w1 w) c -> (b h1 w1) h w c', 'from': (2, 10, 10, 3), 'to': TensorShape([8, 5, 5, 3]), 'equal': True}
{'pattern': 'b (h h1) (w w1) c -> b h w (c h1 w1)', 'from': (2, 10, 10, 3), 'to': TensorShape([2, 5, 5, 12]), 'equal': True}

Other

TODO:

  • Write tests
  • Performance benchmark

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 93.40659% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.98%. Comparing base (ab3c8f5) to head (90078cf).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
keras/src/ops/einops.py 94.38% 4 Missing and 1 partial ⚠️
keras/api/_tf_keras/keras/ops/__init__.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #20733      +/-   ##
==========================================
+ Coverage   81.95%   81.98%   +0.02%     
==========================================
  Files         553      554       +1     
  Lines       51446    51549     +103     
  Branches     7957     7972      +15     
==========================================
+ Hits        42164    42262      +98     
- Misses       7346     7347       +1     
- Partials     1936     1940       +4     
Flag Coverage Δ
keras 81.80% <93.40%> (+0.02%) ⬆️
keras-jax 64.09% <92.30%> (+0.04%) ⬆️
keras-numpy 58.95% <92.30%> (+0.05%) ⬆️
keras-openvino 29.96% <92.30%> (+0.12%) ⬆️
keras-tensorflow 64.75% <93.40%> (+0.04%) ⬆️
keras-torch 64.13% <92.30%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

keras/src/ops/einops.py Outdated Show resolved Hide resolved
keras/src/ops/einops.py Outdated Show resolved Hide resolved
keras/src/ops/einops.py Outdated Show resolved Hide resolved


@keras_export("keras.ops.rearrange")
def rearrange(tensor, pattern, **axes_lengths):
Copy link
Collaborator

Choose a reason for hiding this comment

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

An op should be able to run on either symbolic Keras tensors or backend native eager tensors. And they should render as a single node in the op graph. This would require creating a class for the op, with a compute_output_spec method (see how other ops are implemented)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, sorry, forgot to add the class.

@DavidLandup0
Copy link
Contributor Author

@fchollet Thank you for the feedback! Changes should be reflected in the latest commit.
I'll add more detailed test cases and a performance benchmark still. Sorry for the delay on those.

from keras.src.ops.operation import Operation


def _create_axes_map(axes, input_shape, axes_lengths):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want any documentation or code comments on these?

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

keras/src/ops/einops.py Outdated Show resolved Hide resolved
keras/src/ops/einops.py Outdated Show resolved Hide resolved
**axes_lengths: Keyword arguments specifying lengths of axes
when axes decomposition is used.

Returns:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also add a code example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added examples to mirror: https://einops.rocks/api/rearrange/

output_shape = _compute_output_shape(axes_map, grouped_output_axes)
tensor = reshape(tensor, output_shape)

return tensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's unusual to inline logic in a src/ops/ op rather than defining it N times in the backends in a backend specific fashion. But it's done for a couple other ops (image ops in particular). It's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was debating opening N backend operations instead of one here. Though, since it just uses reshape() and transpose(), it gets to use backend-equal implementations by virtue of keras.ops by default. Figured that lower redundancy/copying is preferred in this case, especially since we could look into adding more operations in keras.src.ops.einops in the future.

@@ -0,0 +1,24 @@
import numpy as np
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add this file to the list of excluded test files for openVINO, or otherwise fix the test. OpenVINO tests are failing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed numpy in lieu of keras.ops - some of the ops in the tests themselves (all(), etc.) don't seem to be supported by openVINO. Skipped those.

@DavidLandup0 DavidLandup0 requested a review from fchollet January 13, 2025 03:43
Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Jan 15, 2025
@fchollet fchollet merged commit 617b821 into keras-team:master Jan 15, 2025
7 checks passed
@google-ml-butler google-ml-butler bot removed awaiting review ready to pull Ready to be merged into the codebase kokoro:force-run labels Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

[Keras Ops] einops.rearrange() Equivalent
4 participants