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

[Bugfix][Relay][Keras] Fix the wrong implementation logic about cropping2D #15053

Merged
merged 10 commits into from
Jun 15, 2023

Conversation

jikechao
Copy link
Contributor

@jikechao jikechao commented Jun 7, 2023

This PR fixed the wrong calculation logic which will lead to wrong inference results.
The original cropping algorithm shown below is incorrect.
image
I modified the code to fix it. Meanwhile, a test case that can trigger this bug was added to the unit test.

Step to reproduce

import tvm
import tvm.relay as relay
import numpy as np
from tensorflow import keras
from tensorflow.keras import layers, models
input_shape = (2, 9, 9, 2)
input_data = np.random.random(size=input_shape)
x = layers.Input(shape=input_shape[1:], dtype='float32')

layer = keras.layers.Cropping2D(cropping=[2,1], data_format='channels_last')
layer.set_weights(layer.get_weights())

y = layer(x)
model = models.Model(x, y)
model.summary()
res_keras = model(input_data)

shape_dict = {'input_1': input_shape}
mod, params = relay.frontend.from_keras(model, shape_dict)
with tvm.transform.PassContext(opt_level=3):
    model = relay.build_module.create_executor("graph", mod, tvm.cpu(0), 'llvm', params).evaluate()

test_x_tvm = input_data
res_tvm = model(tvm.nd.array(test_x_tvm.astype('float32'))).numpy()

np.testing.assert_allclose(res_keras, res_tvm, atol=1e-3, rtol=1e-3)

image

cc @echuraev @masahi @Hzfengsy

jikechao added 2 commits June 7, 2023 21:59
The implementation of cropping2D is wrong. This pr fix it.
@tvm-bot
Copy link
Collaborator

tvm-bot commented Jun 7, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@github-actions github-actions bot requested review from Hzfengsy, echuraev and masahi June 7, 2023 14:13
@jikechao

This comment was marked as resolved.

@masahi
Copy link
Member

masahi commented Jun 8, 2023

See #14983 (comment) for the arm build issue.

@masahi
Copy link
Member

masahi commented Jun 10, 2023

@tvm-bot rerun

@jikechao
Copy link
Contributor Author

The initial patch will lead to existing test cases crashing. So, I updated the patch to fix it.

@jikechao

This comment was marked as duplicate.

2 similar comments
@jikechao
Copy link
Contributor Author

@tvm-bot rerun

@jikechao
Copy link
Contributor Author

@tvm-bot rerun

@jikechao
Copy link
Contributor Author

@echuraev @masahi CI test has finished. Could you help me to merge this PR? Thanks!

@echuraev echuraev merged commit 90b5acc into apache:main Jun 15, 2023
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.

4 participants