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

[Autodiff] Deterministic gradient compute #7321

Merged
merged 11 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/te/autodiff/ad_simplify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,15 +413,17 @@ class FactorOutAtomicFormulasFunctor
auto res_b = VisitExpr(op->b);

// For the And case we return the union of the sets of atomic formulas
std::unordered_set<PrimExpr, StructuralHash, StructuralEqual> res_set;
res_set.reserve(res_a.atomic_formulas.size() + res_b.atomic_formulas.size());
std::unordered_set<PrimExpr, StructuralHash, StructuralEqual> res_a_set;
res_a_set.reserve(res_a.atomic_formulas.size());
std::copy(res_a.atomic_formulas.begin(), res_a.atomic_formulas.end(),
std::inserter(res_set, res_set.end()));
std::copy(res_b.atomic_formulas.begin(), res_b.atomic_formulas.end(),
std::inserter(res_set, res_set.end()));

std::vector<PrimExpr> res{res_set.begin(), res_set.end()};
std::inserter(res_a_set, res_a_set.end()));

std::vector<PrimExpr> res = res_a.atomic_formulas;
for (const auto& e : res_b.atomic_formulas) {
if (res_a_set.find(e) == res_a_set.end()) {
res.emplace_back(e);
}
}
// And the residuals are combined with &&
return {res, res_a.rest && res_b.rest};
}
Expand All @@ -443,32 +445,34 @@ class FactorOutAtomicFormulasFunctor

// For the Or case we intersect the sets of atomic formulas
std::unordered_set<PrimExpr, StructuralHash, StructuralEqual> res_set;
std::vector<PrimExpr> res;
res_set.reserve(std::min(res_a.atomic_formulas.size(), res_b.atomic_formulas.size()));
for (const auto& res_b_formula : res_b_set) {
res.reserve(std::min(res_a.atomic_formulas.size(), res_b.atomic_formulas.size()));
for (const auto& res_b_formula : res_b.atomic_formulas) {
if (res_a_set.count(res_b_formula)) {
res_set.insert(res_b_formula);
res.push_back(res_b_formula);
}
}

// Computing the residual is more complex: we have to compute the sets of atomic formulas
// which are left behind, and then combine them with the residuals into the new residual.
std::vector<PrimExpr> new_cond_a;
new_cond_a.reserve(res_a.atomic_formulas.size() - res_set.size());
for (const auto& formula : res_a_set) {
for (const auto& formula : res_a.atomic_formulas) {
if (!res_set.count(formula)) new_cond_a.emplace_back(formula);
}

std::vector<PrimExpr> new_cond_b;
new_cond_b.reserve(res_b.atomic_formulas.size() - res_set.size());
for (const auto& formula : res_b_set) {
for (const auto& formula : res_b.atomic_formulas) {
if (!res_set.count(formula)) new_cond_b.emplace_back(formula);
}

res_a.atomic_formulas = std::move(new_cond_a);
res_b.atomic_formulas = std::move(new_cond_b);

PrimExpr new_rest = res_a.to_expr() || res_b.to_expr();
std::vector<PrimExpr> res{res_set.begin(), res_set.end()};

return {res, new_rest};
}
Expand Down
16 changes: 16 additions & 0 deletions tests/python/unittest/test_te_autodiff.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,23 @@ def test_reduction_init():
check_grad(B, A0)


def test_stable():
X = te.placeholder((32, 512, 16, 16), name="X")
W = te.placeholder((1024, 512, 1, 1), name="W")
strides, padding, dilation = 2, 0, 1
R = topi.nn.conv2d(X, W, strides, padding, dilation)
ones = topi.full_like(R, 1.0)
grads = te.gradient(R, [X], head=ones)
dag = tvm.auto_scheduler.ComputeDAG(grads)
repeat = 100
for i in range(repeat):
grads = te.gradient(R, [X], head=ones)
new_dag = tvm.auto_scheduler.ComputeDAG(grads)
assert str(dag) == str(new_dag)
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Since auto_scheduler guarantees the DAG would be the same with the same given compute, you don't need to involve auto_scheduler in this test.
  2. I'm even not sure if we need this test because it seems cannot expose the real problem. IIUC, the non-deterministic behavior comes from the use of unordered_set, so you may still pass this pass when you're lucky even you break something. If that happens, this test becomes flaky. But I'd like to hear opinions from others. cc @yzhliu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since auto_scheduler guarantees the DAG would be the same with the same given compute, you don't need to involve auto_scheduler in this test.

Yes, I agree. I use auto_scheduler here only because it provides a hash_key for TE level ir. Any ideas about how to compare two Tensor?

I'm even not sure if we need this test because it seems cannot expose the real problem. IIUC, the non-deterministic behavior comes from the use of unordered_set, so you may still pass this pass when you're lucky even you break something. If that happens, this test becomes flaky. But I'd like to hear opinions from others.

Agree. I'm fine with removing the test.



if __name__ == "__main__":
test_basic_operation()
test_topi()
test_stride_dilation()
test_stable()