[GitHub] [incubator-tvm] jcf94 commented on pull request #6184: [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy

[GitBox]

jcf94 commented on pull request #6184:
URL: https://github.com/apache/incubator-tvm/pull/6184#issuecomment-671763210


   > 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.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [incubator-tvm] jcf94 commented on pull request #6184: [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy [WIP]

[GitBox]

jcf94 commented on pull request #6184:
URL: https://github.com/apache/incubator-tvm/pull/6184#issuecomment-669172637


   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.
   cc @merrymercy @comaniac @FrozenGene @junrushao1994 



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [incubator-tvm] jcf94 commented on pull request #6184: [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy [WIP]

[GitBox]

jcf94 commented on pull request #6184:
URL: https://github.com/apache/incubator-tvm/pull/6184#issuecomment-667757439


   > @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.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [incubator-tvm] jcf94 commented on pull request #6184: [Ansor][AutoTVM v2.0] Phase 2: Basic CPU Sketch Search Policy [WIP]

[GitBox]

jcf94 commented on pull request #6184:
URL: https://github.com/apache/incubator-tvm/pull/6184#issuecomment-667514867


   @merrymercy I realized that a basic 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.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org