-
Notifications
You must be signed in to change notification settings - Fork 209
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
Add support for comparing graphs #85
Conversation
b93aced
to
63fa30a
Compare
d897f62
to
99d06a8
Compare
@neild for review. I still need to add a lot more unit tests. |
cmp/compare.go
Outdated
return | ||
} | ||
defer s.curPtrs.Pop(vx, vy) | ||
*/ |
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.
Drop commented-out code here.
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.
Done.
cmp/compare.go
Outdated
// Report the entire slice as is if the arrays are of primitive kind, | ||
// and the arrays are different enough. | ||
// Checking element pointers is necessary for slices of non-primitives, | ||
// as they may contain slices that sub-slices some parent slice. |
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.
typo: s/sub-slices/sub-slice/
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.
Done.
cmp/path.go
Outdated
// by checking whether the pointer has already been visited. The cycle detection | ||
// uses a seperate stack for the x and y values. | ||
// | ||
// Suppose a cycle were detected, we need to determine whether the two pointers |
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.
"If a cycle is detected we need to..."
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.
Done.
cmp/path.go
Outdated
} | ||
|
||
// Push indicates intent to descend into pointers vx and vy where | ||
// visited reports whether thay have been seen before. If visited before, |
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.
reports whether either has been seen before
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.
Done.
88cfca2
to
132fdc6
Compare
@dsnet , it have been a while 😄 , any intentions to merge this? |
Apologies for the massive delay. This is next on my TODO list in regards to |
Any updates on this ? |
6b9e0db
to
87e1c3f
Compare
Previously, trying to call Equal on a graph would result in a stack-overflow due to infinite recursion traversing cycles on a graph. While a vast majority of Go values are trees or acyclic graphs, there exist a small number of cases where graph equality is required. As such, we add cycle detection to Equal and define what it means for two graphs to be equal. Contrary to reflect.DeepEqual, which declares two graphs to be equal so long any cycle were encountered, we require two graphs to have equivalent graph structures. Mathematically speaking, a graph G is a tuple (V, E) consisting of the set of vertices and edges in that graph. Graphs G1 and G2 are equal if V1 == V2, E1 == E2, and both have the same root vertex (entry point into the graph). When traversing G1 and G2, we remember a stack of previously visited edges ES1 and ES2. If the current edge e1 is in ES1 or e2 is in ES2, then we know that a cycle exists. The graphs have the same structure when the previously encountered edge ep1 and ep2 were encountered together. Note that edges and vertices unreachable from the root vertex are ignored. Appreciation goes to Eyal Posener (@posener), who proposed a different (but semantically equivalent) approach in #79, which served as inspiration. Fixes #74
Previously, trying to call Equal on a graph would result in a stack-overflow
due to infinite recursion traversing cycles on a graph.
While a vast majority of Go values are trees or acyclic graphs, there exist
a small number of cases where graph equality is required.
As such, we add cycle detection to Equal and define what it means for two graphs
to be equal. Contrary to reflect.DeepEqual, which declares two graphs to be
equal so long any cycle were encountered, we require two graphs to have
equivalent graph structures.
Mathematically speaking, a graph G is a tuple (V, E) consisting of the set of
vertices and edges in that graph. Graphs G1 and G2 are equal if V1 == V2,
E1 == E2, and both have the same root vertex (entry point into the graph).
When traversing G1 and G2, we remember a stack of previously visited edges
ES1 and ES2. If the current edge e1 is in ES1 or e2 is in ES2, then we know that
a cycle exists. The graphs have the same structure when the previously
encountered edge ep1 and ep2 were encountered together.
Note that edges and vertices unreachable from the root vertex are ignored.
Appreciation goes to Eyal Posener (@posener), who proposed a different
(but semantically equivalent) approach in #79, which served as inspiration.
Fixes #74