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] Fix the FoldConstant Regression for VTA #6377

Merged
merged 2 commits into from
Sep 2, 2020
Merged

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Sep 2, 2020

Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. I can confirm that this patch causes the VTA end to end test to pass locally.

@tmoreau89
Copy link
Contributor

For reference here's the PR that introduced the regression. The regression was not caught because the of a buggy CI that did not catch the regression, until recently when Tianqi fixed the CI script.

@tmoreau89
Copy link
Contributor

#6195

@anijain2305
Copy link
Contributor

anijain2305 commented Sep 2, 2020

Thanks @tqchen for the fix. Thanks @tmoreau89 for raising this.

The fix ensures that the compilation time improvement is retained from the origin PR #6195

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.

I see...LGTM!

This PR fixes an error guard during the documentation build step.

- Temporary disables VTA frontend tutorial due to
  the regression of deploy_detection
@@ -230,7 +216,17 @@ class ConstantFolder : public ExprMutator {
auto entry_func = Downcast<Function>(mod->Lookup("main"));
expr = expr.as<FunctionNode>() == nullptr ? entry_func->body : entry_func;

FInterpreter executor = GetInterpreter(mod);
using tvm::transform::PassContext;
Copy link
Member

Choose a reason for hiding this comment

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

Why inline this back in?

Copy link
Member Author

Choose a reason for hiding this comment

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

We will need the make sure the context is set during the executor(input) call besides the creation.

@tqchen tqchen merged commit 693c0de into apache:master Sep 2, 2020
kevinthesun pushed a commit to kevinthesun/tvm that referenced this pull request Sep 17, 2020
* [RELAY] Fix the FoldConstant Regression for VTA

* [CI] Fix error guard that was missed in VTA.

This PR fixes an error guard during the documentation build step.

- Temporary disables VTA frontend tutorial due to
  the regression of deploy_detection
kevinthesun pushed a commit to kevinthesun/tvm that referenced this pull request Sep 18, 2020
* [RELAY] Fix the FoldConstant Regression for VTA

* [CI] Fix error guard that was missed in VTA.

This PR fixes an error guard during the documentation build step.

- Temporary disables VTA frontend tutorial due to
  the regression of deploy_detection
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Sep 18, 2020
* [RELAY] Fix the FoldConstant Regression for VTA

* [CI] Fix error guard that was missed in VTA.

This PR fixes an error guard during the documentation build step.

- Temporary disables VTA frontend tutorial due to
  the regression of deploy_detection
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