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

Dont create duplicate nodes in graph merges #4

Open
vadixidav opened this issue Nov 10, 2019 · 0 comments
Open

Dont create duplicate nodes in graph merges #4

vadixidav opened this issue Nov 10, 2019 · 0 comments

Comments

@vadixidav
Copy link
Member

vadixidav commented Nov 10, 2019

deep/deep/src/lib.rs

Lines 79 to 85 in 7cc85ca

fn merge(&mut self, other: Graph) {
let current = self.ops.len();
self.ops.extend(other.ops);
for op in &mut self.ops[current..] {
op.shift_inputs(current);
}
}

In this code, if a graph merges twice (the same tensor is used in two places) then it will create two separate trainable graphs where each one only gets the gradient from one of the places its used. To avoid this, any time a graph is modified, we should create a random unique identifier of every range of ops that was previously part of an older graph. These ranges should be preserved when merging graphs as well. If the unique identifier from a range is found in a graph that is being merged, we should reuse that part of the graph by adding its starting offset minus the starting offset in the other graph. This will allow us to create arbitrary DAGs (directed acyclic graphs) without issue.

This wont surface as an issue if a piece of graph is never used in two different places, which is actually surprisingly rare. This issue is just to track this problem to avoid it randomly appearing unexpectedly.

@vadixidav vadixidav mentioned this issue Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant