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

[MetaSchedule][Refactor] Clarify Integration Logic #10927

Merged

Conversation

junrushao
Copy link
Member

Based on @masahi's PR #10578, we are able to further simplify the logic and remove redundancy.

Changes:

  • Remove MetaScheduleContext, which is not useful any more after Masa's new relay backend;
  • Consolidate logic around ApplyHistoryBest, and slightly improve the error message;
  • Separate integration.py into 3 files: apply_history_best.py, extracted_task.py and relay_integration.py;

These changes make future Relax integration easier - the only thing needed is to add relax_integration.py without having to worry about potential conflicts.

@junrushao
Copy link
Member Author

junrushao commented Apr 7, 2022

@junrushao junrushao force-pushed the feature/2022-04-06/refactor-integration branch 2 times, most recently from 58e5af3 to ceb2bc2 Compare April 7, 2022 06:40
@junrushao junrushao force-pushed the feature/2022-04-06/refactor-integration branch from ceb2bc2 to ab67e5f Compare April 7, 2022 14:43
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! :)

@sunggg
Copy link
Contributor

sunggg commented Apr 7, 2022

Overall, LGTM.
One quick question - it seems like this PR replaces MetaScheduleContext to ApplyHistoryBestContext?
Does it mean other functionalities, like tuning, does not need any contextual information?

@junrushao
Copy link
Member Author

@sunggg Thanks for asking!

There is no breaking change in terms of API usage, given ApplyHistoryBest is the only framework-facing integration point. It's still used in the same way like:

with ApplyHistoryBest(database=...):
   ...

MetaScheduleContext is no longer in use after @masahi's refactoring in both Relay (#10578) and Relax (tlc-pack/relax#92), so it makes sense to just remove it

@junrushao
Copy link
Member Author

Does it mean other functionalities, like tuning, does not need any contextual information?

@sunggg For now, MetaScheduleContext doesn't support providing arbitrary contextual information, so removing it doesn't bring in any regression. In the future, we will need to think very seriously about what the best API design is in terms of supplying arbitrary contextual information

@sunggg
Copy link
Contributor

sunggg commented Apr 7, 2022

@junrushao1994 Thank you for the explanation. Makes sense to me.

Copy link
Member

@zxybazh zxybazh 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 for the refactor!

@masahi masahi merged commit 5f1f8f3 into apache:main Apr 7, 2022
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Apr 11, 2022
Lucien0 pushed a commit to Lucien0/tvm that referenced this pull request Apr 19, 2022
altanh pushed a commit to altanh/tvm that referenced this pull request Apr 28, 2022
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.

6 participants