-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Tree isomorphism #1811
base: main
Are you sure you want to change the base?
Tree isomorphism #1811
Conversation
|
||
|
||
def tree_is_isomorphic(T1, T2): | ||
if not T2: |
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.
Might be more readable as if not T1 and not T2: return True
.
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.
But the suggestion is not the same meaning....
Original PR returns False but suggestion only returns True.
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.
I thought the intention (not the implementation) was that when both are empty, immediately return True
, otherwise run the algorithm.
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.
It was to handle the case when T2 is empty (and so there is no center).
Should two empty graphs be considered as isomorph? Because nx.is_tree(nx.Graph()) raises NetworkXPointlessConcept, but nx.is_isomorphic(nx.Graph(), nx.Graph())
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.
Hm, I believe that two null graphs should be considered isomorphic. The definition of isomorphic for graphs with node sets U and V is "there exists a function f from U to V such that f is a bijection and f preserves edges". Since both U and V are empty, there is just one function from U to V, the empty function, and it is a bijection. Furthermore, the edge preservation requirement is vacuously true.
However, the behavior of the tree isomorphism function should be "undefined" for non-tree inputs (that is, anything can happen). I think that, as with the bipartite
package, the responsibility is on the calling code to decide whether the given graphs are actually trees before calling this is_isomorphic
function. In other words, a pre-condition for calling this function will be that each graph is a tree. Since an empty graph is not a tree, you can assume that you will always get non-empty graphs as input.
For #1602 . I'd just like to have an early feedback, I know it still misses handling eventual corner cases, tests and docs.