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 CPU Sketch Search Policy #6184

Merged
merged 10 commits into from
Aug 11, 2020

Conversation

jcf94
Copy link
Contributor

@jcf94 jcf94 commented Aug 1, 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 CPU 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:

  • Update the API of auto_scheduler::AutoSchedule, TuningOptions and SearchPolicy::Search;
  • Some API name refine for compute_dag.h/cc, cost_model.h/cc;
  • Add search_policy/sketch_search_policy.h/cc, search_policy/utils.h/cc, these're the key parts of this PR;

In the SketchSearchPolicy, we can start the review from SketchSearchPolicyNode::Search and SketchSearchPolicyNode::SearchOneRound, which are the main entrance.

SketchGenerationRule and InitPopulationRule are used to generate the schedule automatically.

Some methods used by the SketchSearchPolicy are put on the search_policy/utils.h/cc.

@jcf94
Copy link
Contributor Author

jcf94 commented Aug 1, 2020

@merrymercy I realized that a base class of CostModel is also needed in this PR. We can sync that part first when you start to work on the cost model PR(Or have a separate PR for the basic random model support?).

Another problem is that I found the setting of random seed does not work on the current implementation of random number generation in RandomModel(neither the python np.random, nor the C++ rand_r).

@merrymercy
Copy link
Member

@jcf94 I sent a separate PR for the base class for cost models (#6187).
Why do we need to rely on the random seed here?

@jcf94
Copy link
Contributor Author

jcf94 commented Aug 3, 2020

@jcf94 I sent a separate PR for the base class for cost models (#6187).
Why do we need to rely on the random seed here?

Thanks!
The random seed issue does not block anything. I addressed it here is just to make sure we're able to fully reproduce a search process. The current UT generates different states in each run.

@jcf94 jcf94 force-pushed the sketch_search_policy branch 3 times, most recently from d1a5777 to 510339c Compare August 5, 2020 12:37
@jcf94 jcf94 marked this pull request as ready for review August 5, 2020 12:42
@jcf94
Copy link
Contributor Author

jcf94 commented Aug 5, 2020

This PR shares the base class with cost models, will rebase the code after #6187 has been merged.
The other parts of code is ready for review.
Rebased with #6187, ready for review.
cc @merrymercy @comaniac @FrozenGene @junrushao1994 @tqchen

@jcf94 jcf94 changed the title [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy [WIP] [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy Aug 6, 2020
@jcf94 jcf94 force-pushed the sketch_search_policy branch from a232b40 to e625554 Compare August 7, 2020 02:41
include/tvm/auto_scheduler/search_policy.h Outdated Show resolved Hide resolved
src/auto_scheduler/compute_dag.cc Show resolved Hide resolved
src/auto_scheduler/search_policy/utils.cc Outdated Show resolved Hide resolved
Copy link
Member

@merrymercy merrymercy left a comment

Choose a reason for hiding this comment

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

LGTM. Only some minor doc improvements.

include/tvm/auto_scheduler/compute_dag.h Show resolved Hide resolved
include/tvm/auto_scheduler/search_policy.h Outdated Show resolved Hide resolved
include/tvm/auto_scheduler/search_policy.h Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/auto_schedule.py Outdated Show resolved Hide resolved
src/auto_scheduler/compute_dag.cc Show resolved Hide resolved
src/auto_scheduler/search_policy/sketch_search_policy.cc Outdated Show resolved Hide resolved
src/auto_scheduler/search_policy/sketch_search_policy.cc Outdated Show resolved Hide resolved
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.

Overall LGTM, but I'd like to raise the discussion about the file organization for search policy. Now sketch_search_policy.cc has about one thousand line and it might continue to grow in the future. Here is the organization in my mind:

auto_scheduler
|- serach_policy
  |- sketch_search
    |- sketch_search_policy.{h,cc}
    |- sketch_rules.{h,cc}
    |- utils.{h,cc}
  |- empty_search
  |- utils.{h,cc}
  • Have auto_scheduler/search_policy/{sketch_policy, empty_policy}.
  • Separate all SketchGenerationRule and InitPopulationRule to search_policy/sketch_policy/sketch_rules.
  • Rename src/auto_scheduler/search_policy/utils.{h,cc} to 'src/auto_scheduler/search_policy/utils.{h,cc}' (still under search_policy), and move all sketch search specific functions such as Mutation (not included in this PR) to auto_scheduler/search_policy/sketch_policy/utils.{h,cc}.

python/tvm/auto_scheduler/auto_schedule.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/auto_schedule.py Outdated Show resolved Hide resolved
python/tvm/auto_scheduler/auto_schedule.py Outdated Show resolved Hide resolved
sch, tensors = _ffi_api.AutoSchedule(task, search_policy,
tuning_options if tuning_options else TuningOptions())
# TODO(jcf94): Remove EmptyPolicy after finish the porting of SketchSearchPolicy
sch, tensors = _ffi_api.AutoSchedule(search_policy or EmptyPolicy(task), tuning_options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make sketch search policy as the default one? Otherwise I can imagine lots of people will ask in the discuss forum about why auto scheduler doesn't do any search...

@jcf94
Copy link
Contributor Author

jcf94 commented Aug 11, 2020

Overall LGTM, but I'd like to raise the discussion about the file organization for search policy. Now sketch_search_policy.cc has about one thousand line and it might continue to grow in the future. Here is the organization in my mind:

auto_scheduler
|- serach_policy
  |- sketch_search
    |- sketch_search_policy.{h,cc}
    |- sketch_rules.{h,cc}
    |- utils.{h,cc}
  |- empty_search
  |- utils.{h,cc}
  • Have auto_scheduler/search_policy/{sketch_policy, empty_policy}.
  • Separate all SketchGenerationRule and InitPopulationRule to search_policy/sketch_policy/sketch_rules.
  • Rename src/auto_scheduler/search_policy/utils.{h,cc} to 'src/auto_scheduler/search_policy/utils.{h,cc}' (still under search_policy), and move all sketch search specific functions such as Mutation (not included in this PR) to auto_scheduler/search_policy/sketch_policy/utils.{h,cc}.

Currently I just split out the sketch_policy_rules.h/cc, we can continue to consider about the directory structure.
We can also put the Evolutionary Search to a separate file.

@jcf94 jcf94 requested review from FrozenGene and merrymercy August 11, 2020 10:38
@merrymercy
Copy link
Member

@comaniac @junrushao1994 Wait for your approval and I will merge this

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.

LGTM

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.

LGTM. The file organization is much clearer now. Thanks :)

@merrymercy merrymercy merged commit 75b8318 into apache:master Aug 11, 2020
@jcf94 jcf94 deleted the sketch_search_policy branch August 12, 2020 01:53
wjliu1998 pushed a commit to wjliu1998/incubator-tvm that referenced this pull request Aug 13, 2020
…#6184)

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

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

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

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

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

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

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

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

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

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

* Init commit to pass the compile

* First commit to pass the test

* Update

* Add UTs for sketch generation

* Update

* Add ASF to new UT file.

* Update rule for winograd

* Update

* File renamed

* Lint fix
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