Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Skip adding control_deps if nodes are the same #8055

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

reminisce
Copy link
Contributor

The following code would fail before this PR due to introducing a cycle into the gradient graph. See this issue for analysis: #8029. This PR also fixes this issue of the same root cause: #7956.

data = mx.symbol.Variable('data')
res = data + data + data + data + data + data + data + data
res.simple_bind(ctx=mx.cpu(), data=(1,))

@piiswrong @tqchen

@tqchen
Copy link
Member

tqchen commented Sep 27, 2017

LGTM, can you backport this to nnvm? Thanks

@piiswrong
Copy link
Contributor

@tqchen Do you remember why the inplace_sum_cap was there? Can we remove it?

@reminisce
Copy link
Contributor Author

@tqchen Could you specify where in nnvm I should backport this fix to?

@tqchen
Copy link
Member

tqchen commented Sep 27, 2017

@reminisce nevermind, this logic is only in mxnet

@tqchen
Copy link
Member

tqchen commented Sep 27, 2017

inplace sum is used to rewrite normal sum to a chain of inplace sum, which saves more space. Since if we do an explicit sum for weight gradient of RNN, we cost O(timestep) memory

@piiswrong piiswrong merged commit 3059030 into apache:master Sep 28, 2017
crazy-cat pushed a commit to crazy-cat/incubator-mxnet that referenced this pull request Oct 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants