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

[FFI] Improve error messages when array/map types do not match in function calls #7330

Merged
merged 6 commits into from
Feb 2, 2021

Conversation

tkonolige
Copy link
Contributor

This PR fixes error messages that arise when a runtime array does not have the correct element types for a function. For example:

# before
Check failed: ObjectTypeChecker<TObjectRef>::Check(ptr) == false: Expect Array[Operation] but get Array
# after
Check failed: ObjectTypeChecker<TObjectRef>::Check(ptr) == false: Expect Array[Operation] but get Array[index 0: Tensor]

Note that because runtime Arrays are not homogenous, specific elements can fail the type checking. I've added which element fails to the errors message along with its type. This can also happen for Maps, but I could not figure out a good way to print which element mismatches (because the elements are unordered and I worry printing the key may take too much space).

This is a replacement for #7309 that returns the mismatched type when a runtime object cannot be converted.

@jroesch @tqchen @junrushao1994 @rkimball

include/tvm/node/container.h Outdated Show resolved Hide resolved
include/tvm/node/container.h Outdated Show resolved Hide resolved
include/tvm/node/container.h Outdated Show resolved Hide resolved
include/tvm/node/container.h Outdated Show resolved Hide resolved
include/tvm/runtime/packed_func.h Outdated Show resolved Hide resolved
@tqchen tqchen changed the title [FIX] Improve error messages when array/map types do not match in function calls [FFI] Improve error messages when array/map types do not match in function calls Jan 24, 2021
include/tvm/node/container.h Outdated Show resolved Hide resolved
include/tvm/node/container.h Outdated Show resolved Hide resolved
src/ir/expr.cc Outdated Show resolved Hide resolved
include/tvm/node/container.h Outdated Show resolved Hide resolved
@junrushao
Copy link
Member

The improvement is super helpful! Thanks!

@tkonolige
Copy link
Contributor Author

I've addressed all the review comments. I totally missed Optional::defined. Its much better than a static_cast. I rename Mismatch to CheckAndGetMismatch and I added the original Checks back in.

@junrushao
Copy link
Member

I think this PR is good to merge once the CI is green

@tkonolige tkonolige force-pushed the array_type_print_v2 branch 2 times, most recently from 77e943a to 75f3a0e Compare January 29, 2021 17:47
@tkonolige
Copy link
Contributor Author

@tqchen @junrushao1994 CI is green for this PR now.

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Thank you @tkonolige for helping improve the error reporting :-)

@junrushao
Copy link
Member

@tqchen Please take another look and approve explicitly if there is no further issue. I can merge it in only with all reviewers' approval :-)

@junrushao junrushao merged commit da42924 into apache:main Feb 2, 2021
alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 11, 2021
…ction calls (apache#7330)

* [FIX] Improve error messages when array/map types do not match in function calls

* missed some places for renaming

* Rename Mismatch to CheckAndGetMismatch. Add Check back in. Use Optional::defined.

* Optional<String> -> String

* formatting

* move ObjectTypeChecker template specializations into where thier respective classes are defined so they will always be found correctly
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
…ction calls (apache#7330)

* [FIX] Improve error messages when array/map types do not match in function calls

* missed some places for renaming

* Rename Mismatch to CheckAndGetMismatch. Add Check back in. Use Optional::defined.

* Optional<String> -> String

* formatting

* move ObjectTypeChecker template specializations into where thier respective classes are defined so they will always be found correctly
Lokiiiiii pushed a commit to Lokiiiiii/tvm that referenced this pull request Mar 2, 2021
…ction calls (apache#7330)

* [FIX] Improve error messages when array/map types do not match in function calls

* missed some places for renaming

* Rename Mismatch to CheckAndGetMismatch. Add Check back in. Use Optional::defined.

* Optional<String> -> String

* formatting

* move ObjectTypeChecker template specializations into where thier respective classes are defined so they will always be found correctly
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Mar 2, 2021
…ction calls (apache#7330)

* [FIX] Improve error messages when array/map types do not match in function calls

* missed some places for renaming

* Rename Mismatch to CheckAndGetMismatch. Add Check back in. Use Optional::defined.

* Optional<String> -> String

* formatting

* move ObjectTypeChecker template specializations into where thier respective classes are defined so they will always be found correctly
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.

3 participants