You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/09/28 22:35:04 UTC
[GitHub] [incubator-tvm] comaniac opened a new pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
comaniac opened a new pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584
As I'm trying to use Ansor to tune the operators generated from `te.gradient`, I fixed some issues in this PR so that now Ansor can tune the backward ops (regardless the performance). Detail change list:
- [Multiple Output] Create a schedule with all outputs instead of the first output.
- [Python API Printing] Change `tvm.thread_axis` to `te.thread_axis`.
- [Python API Printing] Add prefix to all iterators to avoid name conflict; otherwise we may refer to the wrong iterator with the same name hint (e.g., `ax0`). For example:
```python
ax0, ... = tuple(pad_temp_data_grad.op.axis) + ...
...
ax0, ... = tuple(pad_temp_shared.op.axis) + ...
s[pad_temp_data_grad].split(ax0, factor=4)
```
where `ax0` in `split` should refer to the `ax0` from `pad_temp_data_grad`, but it has been overridden by `pad_temp_shared` after `cache_read`. This PR improves `CleanName` by providing an optional prefix so that we can differentiate those iterators by their stages.
cc @merrymercy @jcf94
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#discussion_r496360578
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -969,6 +969,9 @@ void ComputeDAG::RewriteLayout(const Array<Step>& transform_steps) {
}
} // end for placeholder
} // end for stage
+ p_dag->access_analyzer = AccessAnalyzer(p_dag->tensors);
+ p_dag->ops = p_dag->access_analyzer->ops_topo_order;
+ p_dag->flop_ct = FlopEstimator().EstimateFlop(p_dag->ops);
Review comment:
Why did it work before without the fix?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#discussion_r496369109
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -969,6 +969,9 @@ void ComputeDAG::RewriteLayout(const Array<Step>& transform_steps) {
}
} // end for placeholder
} // end for stage
+ p_dag->access_analyzer = AccessAnalyzer(p_dag->tensors);
+ p_dag->ops = p_dag->access_analyzer->ops_topo_order;
+ p_dag->flop_ct = FlopEstimator().EstimateFlop(p_dag->ops);
Review comment:
I would like to know the answer too... For the ApplySteps in compute DAG, previously we simply put all non-placeholder ops to the list and take the last op (assuming it is the only output) to create a new schedule. After using `access_analyzer.is_output` to check what ops should be put to the list, `is_output` throws exceptions due to the out-of-date `is_output` map. That's why I found this issue. Maybe most use cases so far don't enable layout rewrite?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#discussion_r496360088
##########
File path: src/auto_scheduler/search_policy/sketch_policy_rules.h
##########
@@ -74,6 +75,8 @@ class SketchGenerationRule {
*/
virtual std::vector<std::pair<State, int>> Apply(const SketchPolicyNode& policy,
const State& state, int stage_id) const = 0;
+
+ virtual std::string GetRuleName() const = 0;
Review comment:
Document it
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] merrymercy commented on a change in pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#discussion_r496360578
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -969,6 +969,9 @@ void ComputeDAG::RewriteLayout(const Array<Step>& transform_steps) {
}
} // end for placeholder
} // end for stage
+ p_dag->access_analyzer = AccessAnalyzer(p_dag->tensors);
+ p_dag->ops = p_dag->access_analyzer->ops_topo_order;
+ p_dag->flop_ct = FlopEstimator().EstimateFlop(p_dag->ops);
Review comment:
Why does it work before without the fix?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#issuecomment-700353985
Current issue: `te.InferBound` failed for schedules with multiple outputs. Investigating.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#issuecomment-702739318
Thanks @comaniac @merrymercy
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on a change in pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#discussion_r496369109
##########
File path: src/auto_scheduler/compute_dag.cc
##########
@@ -969,6 +969,9 @@ void ComputeDAG::RewriteLayout(const Array<Step>& transform_steps) {
}
} // end for placeholder
} // end for stage
+ p_dag->access_analyzer = AccessAnalyzer(p_dag->tensors);
+ p_dag->ops = p_dag->access_analyzer->ops_topo_order;
+ p_dag->flop_ct = FlopEstimator().EstimateFlop(p_dag->ops);
Review comment:
I wish I know the answer... For the ApplySteps in compute DAG, previously we simply put all non-placeholder ops to the list and take the last op (assuming it is the only output) to create a new schedule. After using `access_analyzer.is_output` to check what ops should be put to the list, `is_output` throws exceptions due to the out-of-date `is_output` map. That's why I found this issue. Maybe most use cases so far don't enable layout rewrite?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-tvm] comaniac commented on pull request #6584: [Ansor] Support multiple output ops and fix Python API printing
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #6584:
URL: https://github.com/apache/incubator-tvm/pull/6584#issuecomment-700370170
Two more bug fixing:
1. Only specify output tensors when creating a new schedule; otherwise InferBound may crash.
2. Update compute DAG attributes such as access analyzer and ops after layout rewriting (cc @minminsun ).
All auto_scheduler tests are now passed.
----------------------------------------------------------------
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:
users@infra.apache.org