Skip to content

[dyn.nn.pad] cast pad value to input dtype#10818

Merged
AndrewZhaoLuo merged 1 commit intoapache:mainfrom
sfvaroglu:sevin/dyn_pad_type
Mar 29, 2022
Merged

[dyn.nn.pad] cast pad value to input dtype#10818
AndrewZhaoLuo merged 1 commit intoapache:mainfrom
sfvaroglu:sevin/dyn_pad_type

Conversation

@sfvaroglu
Copy link
Contributor

We encountered a bug where dyn.nn.pad did not match input dtype. This PR casts the pad value to input dtype to fix the issue.

@sfvaroglu
Copy link
Contributor Author

@mbrookhart @AndrewZhaoLuo

Copy link
Contributor

@AndrewZhaoLuo AndrewZhaoLuo left a comment

Choose a reason for hiding this comment

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

LGTM, can you write a quick test to catch this?

@sfvaroglu
Copy link
Contributor Author

LGTM, can you write a quick test to catch this?

I tried to add a test case in relay but ran into other type issues. Turns out pad_value cannot be a constant (expected to be a variable).

However, the problematic op (e.g. dyn.nn.pad(%282, %291, 0f /* ty=float32 */, pad_width=[]) /* ty=Tensor[(?), int64] */) is from a tflite model (tflite->onnx->relay) with a constant pad_value. Perhaps it doesn't go through the same checks.

@AndrewZhaoLuo
Copy link
Contributor

Interesting, hmm ok fine with skipping unittest for now. These can be annoying.

@AndrewZhaoLuo AndrewZhaoLuo merged commit 21eb583 into apache:main Mar 29, 2022
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
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.

2 participants