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

[Relay] Modify create_executor to pass params #8418

Merged
merged 2 commits into from
Jul 13, 2021

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!

@@ -555,6 +559,9 @@ def create_executor(kind="debug", mod=None, device=None, target="llvm"):
else:
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