-
Notifications
You must be signed in to change notification settings - Fork 118
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
Converted to keras core external attention #529
Converted to keras core external attention #529
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this 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!
""" | ||
## Setup | ||
""" | ||
import keras_core as keras |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the example is TF-only, start by setting the backend using os
|
||
def call(self, images): | ||
batch_size = tf.shape(images)[0] | ||
patches = tf.image.extract_patches( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every TF call except this one can be converted to ops
. This will make the example less TF dependent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe extract patches can be implemented by a convolution, no? It's a convolution with identity kernel. Maybe we could do that and make the example backend-agnostic.
Alternatively, maybe we can add a cross-backend extract_patches function in ops.image
(implemented using convolution for other backends). Actually, this is a better solution, since this method comes up a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can create a PR for it.
Another workaround: For this example can I use frameworks like einops for creating patches?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer using a conv.
I have found this easy way to extract patches. Can you please elaborate/refer some example of using conv. As far as I understand, conv uses reduce sum of a filter. Is there any 'do nothing' filter for convolution that can be used for upsampling? |
Your way is fine. Conv is likely faster on GPU though. It could be worth trying both if we add the op to the API (just for this example we can keep your way since it's concise).
"Do nothing" is just an identity kernel. There is no "sum" operation, it's a matmul. It turns into a sum if your kernel is all ones. |
Here's the initial 'code' version I have got for extracting patches and it is working. Thank you for guidance. Will create a PR for adding this to cross-backend ops.
|
Thanks! Should we wait for this feature before revisiting this example? |
Sure. That'll be better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Update imports
Remove TensorFlow Addons Dependency
Convert TensorFlow ops in functional component to keras_core ops
Attached git-diff
Colab Notebook
eanet_keras_core.diff.txt