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

adding has_subgraph_decomposition method to GenericGraph #39598

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

seblabbe
Copy link
Contributor

Following the question https://ask.sagemath.org/question/81610/test-if-a-graph-has-a-claw-decomposition/,
the current PR is adding a has_subgraph_decomposition method to the GenericGraph class.

It also adds a _subgraph_decomposition_dlx method allowing to enumerate/count the decompositions.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

@seblabbe seblabbe force-pushed the graph_decomposition branch from 041554b to 40cbaf3 Compare February 27, 2025 11:55
@seblabbe seblabbe requested a review from dcoudert February 27, 2025 11:56
@seblabbe
Copy link
Contributor Author

A question I still have is whether a method has_subgraph_decomposition is the best choice. Should it rather be a method subgraph_decomposition returning a decomposition? Should it be a method subgraph_decompositions returning an iterator over all decomposition?

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 27, 2025

While I was writing the code, I observed a bug in the current subgraph search which yield duplicate copies of the same subgraph. This is why I first need to create a set of rows to avoid duplicates.

I created #39599 for this issue.

Copy link

Documentation preview for this PR (built with commit 40cbaf3; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@dcoudert
Copy link
Contributor

Some comments

  • It seems that the method is currently for undirected graphs only. If so, it should be in graph.py or in a new module devoted to subgraph search and subgraph partition/covering.
  • I like the idea of an iterator subgraph_decompositions.
  • I don't see the need for method _subgraph_decomposition_dlx. Unless you plan to implement several methods, it can be included in the main method.
  • Concerning subgraph_search_iterator, I don't know how to avoid returning several times the same graph. If you have an algorithm in mind, we could at its implementation to the list of GSoC'25 project proposals.
  • Add a warning in the method that the memory consumption can be huge, and a todo statement to recall to improve the implementation when subgraph_search_iterator will be improved

sage: dlx, F = G2._subgraph_decomposition_dlx(claw, induced=True)
sage: dlx
Dancing links solver for 0 columns and 0 rows

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line

from sage.combinat.matrices.dancing_links import dlx_solver

edges = list(self.edges(labels=False))
edge_to_column_id = {frozenset(edge):i for (i,edge) in enumerate(edges)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edge_to_column_id = {frozenset(edge): i for I, edge in enumerate(edges)}


EXAMPLES::

sage: from slabbe.graph import has_graph_decomposition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add path in sagemath


sage: G2.has_subgraph_decomposition(claw, induced=True)
False

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line.

may be add some tests when the method is not working ? what if the graph is not simple ?

solution = dlx.one_solution(ncpus=1)
has_solution = not solution is None

if not certificate:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not certificate:
    return has_solution
if has_solution:
    return has_solution, F(solution)
return has_solution, solution

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

Successfully merging this pull request may close these issues.

2 participants