Skip to content

Conversation

@wweic
Copy link
Contributor

@wweic wweic commented Nov 8, 2019

The existing tests should be sufficient to validate this change. It's not easy to craft a unit test case that failed before and pass after this change.

Discussion: https://discuss.tvm.ai/t/tensorarray-globalvar-and-globaltypevar-confusion/4567/17

cc @yongwww @zhiics @icemelon9 @kevinthesun @srkreddy1238

Copy link
Contributor

@kevinthesun kevinthesun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

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

could you also add some test case?

Copy link
Contributor

@soiferj soiferj left a comment

Choose a reason for hiding this comment

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

LGTM. I had missed the strided slice part in my attempt at fixing this 😊 thanks for the fix.

@wweic wweic force-pushed the pass-module-shape-infer branch from 9488d71 to 6751030 Compare November 9, 2019 14:01
@wweic
Copy link
Contributor Author

wweic commented Nov 9, 2019

Seems like the error is unrelated to my change: #4284

@wweic wweic force-pushed the pass-module-shape-infer branch from 6751030 to 03f5174 Compare November 10, 2019 03:21
@wweic
Copy link
Contributor Author

wweic commented Nov 11, 2019

@icemelon9 could you take another look?

@icemelon icemelon merged commit 10b77ef into apache:master Nov 11, 2019
@icemelon
Copy link
Member

Thanks @wweic

@wweic wweic deleted the pass-module-shape-infer branch November 11, 2019 22:06
zxy844288792 pushed a commit to neo-ai/tvm that referenced this pull request Nov 13, 2019
* [TF][Relay][Op] Pass module when infer shape

* Fix lint

* Improve style

* Add test
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.

5 participants