Skip to content

Conversation

@yaoyaoding
Copy link
Contributor

Hi community,

This PR fixes a bug in binding params for relay function. When we bind constant params for relay function, we hope to report the duplicated param names that we want to bind, but the current implementation does not. In the demo code:

from tvm import relay

a = relay.var('a', shape=(1,))
aa = relay.var('a', shape=(1,))
s = a + aa
func = relay.Function([a, aa], s)
func = relay.build_module.bind_params_by_name(func, {'a': [1.0]})

there are two variables with duplicated names, but we can bind the value to the first 'a'.

With this PR, we will have the following error message:

Traceback (most recent call last):
  File "demo.py", line 7, in <module>
    func = relay.build_module.bind_params_by_name(func, {'a': [1.0]})
  File "/home/yaoyao/repos/tvm/python/tvm/relay/build_module.py", line 449, in bind_params_by_name
    return _build_module.BindParamsByName(func, inputs)
  File "/home/yaoyao/repos/tvm/python/tvm/_ffi/_ctypes/packed_func.py", line 237, in __call__
    raise get_last_ffi_error()
tvm._ffi.base.TVMError: Traceback (most recent call last):
  2: TVMFuncCall
  1: std::_Function_handler<void (tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*), tvm::relay::backend::{lambda(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*)#2}>::_M_invoke(std::_Any_data const&, tvm::runtime::TVMArgs&&, tvm::runtime::TVMRetValue*&&)
  0: tvm::relay::backend::BindParamsByName(tvm::relay::Function, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, tvm::runtime::NDArray, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, tvm::runtime::NDArray> > > const&)
  File "/home/yaoyao/repos/tvm/src/relay/backend/utils.h", line 343
TVMError: Multiple args in the function have name a

@icemelon @comaniac @jroesch

@junrushao
Copy link
Member

Hey thanks @yaoyaoding! Would you like to add a unittest? You may use pytest.raises to assert the error message

@yaoyaoding
Copy link
Contributor Author

Sure, do you have any suggestion about where to put the test?

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM. Please fix the CI.

@junrushao junrushao merged commit bdb311b into apache:main Oct 24, 2021
@yaoyaoding yaoyaoding deleted the fix_bind_params branch October 24, 2021 22:50
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
…rams (apache#9350)

* [Fixbug] Report duplicated param names of relay function when bind params

* add test

* lint
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…rams (apache#9350)

* [Fixbug] Report duplicated param names of relay function when bind params

* add test

* lint
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