Skip to content

Conversation

@mikepapadim
Copy link
Contributor

@mikepapadim mikepapadim commented Jul 7, 2021

Following the forum discussion (https://discuss.tvm.apache.org/t/questions-about-tvm-executors-and-its-apis/10289/3) and the tutorials https://tvm.apache.org/docs/tutorials/frontend/from_keras.html#sphx-glr-tutorials-frontend-from-keras-py and https://tvm.apache.org/docs/tutorials/frontend/from_onnx.html#compile-the-model-with-relay there is some performance mismatch between the two apis.

This repo captures the performance issue.

It seems when one uses the relay API as in the example below, params are not passed properly to the TIR and all optimizations regarding constants (i.e., constant folding) are prevented. Therefore, this leads to a performance mismatch with the relay.vm.compile(mod, target=target, params=params) .

Example usage:

vm_executor = relay.create_executor("vm", mod, tvm.cpu(0), target)
vm_executor.evaluate()(input_data, **params)

This PR modifies the above to accept:

vm_executor = relay.create_executor("vm", mod, tvm.cpu(0), target, params)
vm_executor.evaluate()(input_data)

With this modification relay.vm.compile and relay.create_executor generate the same bytecodes while applying the same opts.

@jroesch @tqchen @YuchenJin @ZihengJiang

@mikepapadim mikepapadim force-pushed the executor_api_fix branch 2 times, most recently from 2a937d8 to 9bc6874 Compare July 7, 2021 16:19
Copy link
Contributor

@YuchenJin YuchenJin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @mikepapadim for the contribution!

device = _nd.device(str(target), 0)

if params is not None:
mod = IRModule.from_expr(bind_params_by_name(mod["main"], params))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to consider what if the mod does not contain a "main" function, or the mod contains multiple functions(subgraphs), each with a different params dict?

Copy link
Contributor

@YuchenJin YuchenJin Jul 13, 2021

Choose a reason for hiding this comment

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

I think it's fine to throw an error if cannot find "main" in the Module, so ignore this comment. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and it is passing all the CI stages.

@ZihengJiang ZihengJiang merged commit 957cc12 into apache:main Jul 13, 2021
@ZihengJiang
Copy link
Contributor

Merged now. Thanks @mikepapadim

ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
* Overload create_executor to accept params

* [fix] Add stringdoc for new param in create_executor
zxy844288792 pushed a commit to zxy844288792/tvm that referenced this pull request Mar 4, 2022
* Overload create_executor to accept params

* [fix] Add stringdoc for new param in create_executor
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