-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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: Evolutionary Search #6310
Conversation
de6e232
to
6cf122b
Compare
c4ff93f
to
1eeae6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me.
tests/python/unittest/test_auto_scheduler_evolutionary_search.py
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
*state = policy->search_task->compute_dag.InferBound(*state); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This InferBound is required during SampleInitPopulation. However, it introduces a redundant InferBound during EvolutionarySearch. We should remove it during EvolutionarySearch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Miss this line when moving the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I realized that it's hard to recognize if this is being used by initial population or mutation. Do you have any suggestions? Or should we just simply remove this mutation in the initial population as @jcf94 suggested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@comaniac We should not remove it. I think it is useful in SampleInitPopulation and further improvement can be done here to make the initial population better.
To solve the issue, we can create a new function for the common part and create two separate rules.
On rule only calls the common part, while the other rule calls the common part + InferBound.
755d1b2
to
2b7a3f8
Compare
3c24580
to
ee26b83
Compare
The test case looks good to me. The two remaining items:
|
@merrymercy comment addressed. PTAL. |
* init commit * Add rest rules * refactor * address comments * improve test * address comments
* init commit * Add rest rules * refactor * address comments * improve test * address comments
* init commit * Add rest rules * refactor * address comments * improve test * address comments
For the full upstream plan, see Ansor RFC.
This PR adds evolutionary search implementations to the auto-scheduler:
cc @merrymercy @jcf94 @tqchen. The checked 3 parts are ready to be reviewed. It's also fine if you guys prefer to put the rest mutation rules to a separate PR.