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

[Ansor][AutoTVM v2.0] Phase 2: Basic GPU Sketch Search Policy #6269

Merged
merged 17 commits into from
Aug 24, 2020

Conversation

jcf94
Copy link
Contributor

@jcf94 jcf94 commented Aug 13, 2020

For full upstream plan, see Ansor RFC.

In this PR, we bring the basic Sketch search policy which is proposed in our paper on GPU ops/subgraphs.
The complete search policy still waits for the feature extraction & cost model support, so this will be a basic search policy using random cost model.

We'll continue to work on the upstreaming process to bring the complete auto schedule support for various workloads on CPU/GPU with some reproducible performance results, a custom sketch rule support to work like the AutoTVM, as well as the tutorials of how to play with AutoScheduler.

Review guide

Main changes:

  • Add PreloadMeasuredStates
  • Split the Python API of search_policy to a separate file search_policy.py
  • Bug fix on auto_scheduler.auto_schedule returning empty state
  • Bug fix for measure record. The former implementation used the default std::vector<int> handler, which output multiple lines when the array size is larger than 10
  • A brief tutorial for CPU that used RandomModel, the performance is not guaranteed since currently our cost model support has not finished
  • Use macro definition to simplify the sketch_policy_rules.h
  • The most important part: Add extra rules for GPU policy

cc @merrymercy @FrozenGene @minminsun @tqchen @junrushao1994

@merrymercy
Copy link
Member

merrymercy commented Aug 13, 2020

Please remove the tutorial. It is better to make the tutorials as a single PR, so people can review it easily and give more suggestions on the API design. It is not good to leave "todo" in the tutorial.

I think the current API still has room for improvement and I will work on it later.

@jcf94
Copy link
Contributor Author

jcf94 commented Aug 13, 2020

Please remove the tutorial. It is better to make the tutorials as a single PR, so people can review it easily and give more suggestions on the API design. It is not good to leave "todo" in the tutorial.

I think the current API still has room for improvement and I will work on it later.

Fine, these modifications are removed.

@CyanFi
Copy link

CyanFi commented Aug 14, 2020

I think there is a typo in the comments here
line 287 should be "Skip the rest rules" instead of "Skip the reset rules"

@jcf94
Copy link
Contributor Author

jcf94 commented Aug 14, 2020

I think there is a typo in the comments here
line 287 should be "Skip the rest rules" instead of "Skip the reset rules"

Thanks, you're right.
You can write your review comments on the code directly. : )

python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/search_policy.py Outdated Show resolved Hide resolved
src/auto_scheduler/search_policy/utils.h Outdated Show resolved Hide resolved
src/auto_scheduler/search_task.cc Outdated Show resolved Hide resolved
@merrymercy merrymercy self-assigned this Aug 19, 2020
@merrymercy
Copy link
Member

cc @comaniac @Laurawly

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.

Just minor comments. Also I think #6310 should be based on this PR as they share some utilities and structures. I'll rebase it before marking it ready for review after this PR is merged.

@@ -73,84 +73,51 @@ class SketchGenerationRule {
const State& state, int stage_id) const = 0;
};

#define DEFINE_SKETCH_GENERATION_RULE(rule_name) \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to raise a discussion about using macro here. Although we can of course largely reduce the code size with this macro, it makes code tracing more difficult. For example, most IDE navigation cannot locate class RuleSkipStage anymore. If we want to avoid redundant lines of

std::vector<std::pair<State, int>> Apply(const SketchPolicyNode& policy, const State& state,	
                                           int stage_id) const final;

maybe we can make it not a virtual function so that we can simply override it in the cc file without declaration in .h file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have clear idea on this part, but I think it's fine to try your suggestion.
Simply merged this PR first, so it will be more convenient for you to try different class structures in your PR.

@FrozenGene FrozenGene merged commit b1f8f15 into apache:master Aug 24, 2020
@FrozenGene
Copy link
Member

Thanks @jcf94 @merrymercy @comaniac merged

@jcf94 jcf94 deleted the gpu_search_policy branch August 24, 2020 10:51
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
…#6269)

* Add PreloadMeasuredStates & Split search_policy.py

* Add GPU sketch rule

* Update

* Bug fix for log record

* Lint fix

* Update tutorial

* Update

* UT fix

* Remove tutorial

* Update

* Update

* Update UT

* Lint fix

* Update

* Update
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
…#6269)

* Add PreloadMeasuredStates & Split search_policy.py

* Add GPU sketch rule

* Update

* Bug fix for log record

* Lint fix

* Update tutorial

* Update

* UT fix

* Remove tutorial

* Update

* Update

* Update UT

* Lint fix

* Update

* Update
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
…#6269)

* Add PreloadMeasuredStates & Split search_policy.py

* Add GPU sketch rule

* Update

* Bug fix for log record

* Lint fix

* Update tutorial

* Update

* UT fix

* Remove tutorial

* Update

* Update

* Update UT

* Lint fix

* Update

* Update
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Sep 2, 2020
…#6269)

* Add PreloadMeasuredStates & Split search_policy.py

* Add GPU sketch rule

* Update

* Bug fix for log record

* Lint fix

* Update tutorial

* Update

* UT fix

* Remove tutorial

* Update

* Update

* Update UT

* Lint fix

* Update

* Update
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Sep 3, 2020
…#6269)

* Add PreloadMeasuredStates & Split search_policy.py

* Add GPU sketch rule

* Update

* Bug fix for log record

* Lint fix

* Update tutorial

* Update

* UT fix

* Remove tutorial

* Update

* Update

* Update UT

* Lint fix

* Update

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