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

graph subgraph_search does not return copy of subgraph #12164

Closed
jasongrout opened this issue Dec 16, 2011 · 9 comments
Closed

graph subgraph_search does not return copy of subgraph #12164

jasongrout opened this issue Dec 16, 2011 · 9 comments

Comments

@jasongrout
Copy link
Member

Consider this example:

sage: k3=graphs.CompleteGraph(3)
sage: p3=graphs.PathGraph(3)
sage: s=k3.subgraph_search(p3)
sage: s.size()
3
sage: s==k3
True

The docs say that s should be a copy of p3. The problem is that subgraph_search returns an induced subgraph of the bigger graph with the vertices of the copy of the smaller graph.

apply: attachment: trac-12164-subgraph_search.patch

CC: @nathanncohen @sagetrac-brunellus

Component: graph theory

Author: Jason Grout

Reviewer: Nathann Cohen

Merged: sage-4.8.alpha5

Issue created by migration from https://trac.sagemath.org/ticket/12164

@jasongrout
Copy link
Member Author

comment:2

Steve Butler suggests that instead, we copy the smaller graph and relabel the vertices appropriately, using the vertices returned from SubgraphSearch. I like that solution. But it won't quite work to do that, since a subgraph of a graph inherits lots of properties, like a name, etc.

So instead, this patch just creates a subgraph where the vertices and the edges are specified. Before, just the vertices were specified, so you ended up getting an induced subgraph, even if the subgraph was not an induced subgraph.

@jasongrout

This comment has been minimized.

@jasongrout
Copy link
Member Author

Author: Jason Grout

@jasongrout

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 16, 2011

comment:6

Hellooooooo !! First, I'm glad to see somebody's using that function ! It needs to be totally rewritten anyway, as we now have.... Cython iterators ! O_O

So the whole class has become useless. This being said, what you are doing is much more urgent. Could I ask for a small "if induced" before those lines, so that the graph is copied/relabeled only if it is necessary ? :-)

Nathann

@jasongrout
Copy link
Member Author

comment:7

Attachment: trac-12164-subgraph_search.patch.gz

Done. I also added a few more tests to the documentation (one of which would have caught this error originally---looking for the non-induced P_5_ in the petersen graph.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Dec 16, 2011

comment:8

Then it's good to go ! All tests pass, and there is just nothing to add :-)

Nathann

@jdemeyer
Copy link

Reviewer: Nathann Cohen

@jdemeyer
Copy link

Merged: sage-4.8.alpha5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants