PR for issue 32136: Add parameter 'induced' to `connected_subgraphs_iterator` #35131

dcoudert · 2023-02-14T16:03:50Z

…-induced subgraphs

dcoudert · 2023-02-14T16:17:55Z

This is my first PR on GitHub. Not obvious for dummies...
So far I don't understand how to add labels, but this branch is ready for review.

codecov-commenter · 2023-02-14T19:06:39Z

Codecov Report

Patch coverage: 97.95% and no project coverage change

Comparison is base (52a81cb) 88.57% compared to head (9d177a5) 88.58%.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #35131    +/-   ##
=========================================
  Coverage    88.57%   88.58%            
=========================================
  Files         2140     2140            
  Lines       397273   397415   +142     
=========================================
+ Hits        351891   352052   +161     
+ Misses       45382    45363    -19

Impacted Files	Coverage Δ
src/sage/schemes/elliptic_curves/ell_generic.py	`93.11% <66.66%> (-0.12%)`	⬇️
src/sage/interfaces/tachyon.py	`87.93% <90.00%> (+0.43%)`	⬆️
src/sage/schemes/elliptic_curves/gal_reps.py	`82.23% <90.00%> (+0.04%)`	⬆️
src/sage/quadratic_forms/quadratic_form.py	`90.26% <95.65%> (+0.16%)`	⬆️
src/sage/modular/quasimodform/element.py	`99.20% <100.00%> (+0.06%)`	⬆️
src/sage/rings/qqbar.py	`95.30% <100.00%> (+<0.01%)`	⬆️
src/sage/schemes/affine/affine_morphism.py	`90.33% <100.00%> (ø)`
src/sage/schemes/elliptic_curves/BSD.py	`43.75% <100.00%> (+0.21%)`	⬆️
src/sage/schemes/elliptic_curves/cardinality.py	`87.54% <100.00%> (+0.93%)`	⬆️
src/sage/schemes/elliptic_curves/cm.py	`89.52% <100.00%> (+0.39%)`	⬆️
... and 70 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mkoeppe · 2023-02-14T19:12:34Z

So far I don't understand how to add labels

I've sent you an invite for the team that has the permissions for that

videlec

I do not quite understand why you use here the dense datastructure rather than the sparse one. Since you need to iterate as fast as possible over the list of vertices the latter naively seems more appropriate to me.

videlec · 2023-03-02T19:38:01Z

src/sage/graphs/base/static_dense_graph.pyx

+            # If vertex u has k active neighbors, we have 2^k possibilities. We
+            # use the binary representation of n_cpt[i] to indicate which of the
+            # k neighbors are selected. We omit the empty neighborhood which is
+            # considered else where.


"else where" -> "elsewhere"

Well, this iterator was already using dense data structure, so I added the functionality in it. We can of course try to do the same with another data structure and compare performances, but this should be done in a separate issue/PR.

videlec

Isn't the following naive algorithm (theoretically) optimal

pick an edge e
put e in H, merge its two ends in G and make a recursive call
remove e from G, if the resulting graph is connected make a recursive call

Here by optimal, I mean that it explores no more edges than what is needed. Though, it does not seem delicate to make many efficient calls to connectedness + design an appropriate datastructure with efficient merge/removal.

src/sage/graphs/base/static_dense_graph.pyx

github-actions · 2023-03-04T16:44:57Z

Documentation preview for this PR is ready! 🎉
Built with commit: 9d177a5

dcoudert · 2023-03-04T17:08:17Z

@videlec, I'm not sure what you want to compute with the algorithm you propose. With edge contraction, you may force the selection of some edges. For instance, if you have the triangle [1, 2, 3], after contracting edge (1, 2), you can either take both edges (1, 3) and (2, 3), or none of them. So I think you may miss some non-induced connected subgraphs.

videlec · 2023-03-05T11:00:21Z

@videlec, I'm not sure what you want to compute with the algorithm you propose. With edge contraction, you may force the selection of some edges. For instance, if you have the triangle [1, 2, 3], after contracting edge (1, 2), you can either take both edges (1, 3) and (2, 3), or none of them. So I think you may miss some non-induced connected subgraphs.

You indeed need to keep track edge multiplicities in the contracted graph. Once you contracted e = (1, 2), taking either (1, 3), (2, 3) or both makes no difference in G / e. "Choosing an edge e" should be replaced by "choose a non-empty subset of edges in the multiedge e".

dcoudert · 2023-03-05T11:17:42Z

Does not seems easy to implement. This can be the object of a future PR.

videlec · 2023-03-05T11:29:33Z

Does not seems easy to implement.

Sure. I would be delighted to know if there is a generation algorithm with O(1) delay!

This can be the object of a future PR.

I definitely agree :-)

dcoudert · 2023-03-05T12:40:03Z

I don't think that O(1) delay is possible as you need to check connectivity after each edge deletion.

videlec · 2023-03-05T12:48:31Z

I don't think that O(1) delay is possible as you need to check connectivity after each edge deletion.

Maybe an adaptation of : https://dl.acm.org/doi/10.1145/322234.322235?

dcoudert added 8 commits October 15, 2022 18:42

trac sagemath#32136: add iterator over full subgraphs

9eed85b

trac sagemath#32136: extend method connected_subgraph_iterator to non…

f613661

…-induced subgraphs

trac sagemath#32136: fix doctests

78d9d93

trac sagemath#32136: fix typos

9c3e732

trac sagemath#32136: merged with 9.8.beta2

e8ea54b

trac sagemath#32136: add bounds on the number of edges

720368f

trac sagemath#32136: fix result for digraphs

e870546

sagemath#32136 ; merge with 10.beta0

edcee72

mkoeppe added the c: graph theory label Feb 14, 2023

dcoudert added s: needs review t: enhancement p: minor / 4 labels Feb 14, 2023

videlec reviewed Mar 2, 2023

View reviewed changes

dcoudert added 2 commits March 3, 2023 11:28

Merge branch 'develop' into graphs/32136_subgraph_iterator

d2ff63e

PR sagemath#35131: fix typo

02ab89e

videlec reviewed Mar 3, 2023

View reviewed changes

src/sage/graphs/base/static_dense_graph.pyx Show resolved Hide resolved

src/sage/graphs/base/static_dense_graph.pyx Show resolved Hide resolved

dcoudert added 2 commits March 4, 2023 13:17

PR sagemath#35131: review comments

d86c4e6

PR sagemath#35131: use decomposition into biconnected components

9d177a5

videlec added s: positive review and removed s: needs review labels Mar 5, 2023

videlec approved these changes Mar 5, 2023

View reviewed changes

vbraun merged commit e6bc06f into sagemath:develop Mar 26, 2023

kwankyu removed the s: positive review label Mar 26, 2023

mkoeppe added this to the sage-10.0 milestone Mar 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR for issue 32136: Add parameter 'induced' to `connected_subgraphs_iterator` #35131

PR for issue 32136: Add parameter 'induced' to `connected_subgraphs_iterator` #35131

dcoudert commented Feb 14, 2023 •

edited

Loading

dcoudert commented Feb 14, 2023

codecov-commenter commented Feb 14, 2023 •

edited

Loading

mkoeppe commented Feb 14, 2023

videlec left a comment

videlec Mar 2, 2023

dcoudert Mar 3, 2023

dcoudert Mar 3, 2023

videlec Mar 3, 2023

videlec left a comment

github-actions bot commented Mar 4, 2023

dcoudert commented Mar 4, 2023

videlec commented Mar 5, 2023 •

edited

Loading

dcoudert commented Mar 5, 2023

videlec commented Mar 5, 2023

dcoudert commented Mar 5, 2023

videlec commented Mar 5, 2023

PR for issue 32136: Add parameter 'induced' to connected_subgraphs_iterator #35131

PR for issue 32136: Add parameter 'induced' to connected_subgraphs_iterator #35131

Conversation

dcoudert commented Feb 14, 2023 • edited Loading

dcoudert commented Feb 14, 2023

codecov-commenter commented Feb 14, 2023 • edited Loading

Codecov Report

mkoeppe commented Feb 14, 2023

videlec left a comment

Choose a reason for hiding this comment

videlec Mar 2, 2023

Choose a reason for hiding this comment

dcoudert Mar 3, 2023

Choose a reason for hiding this comment

dcoudert Mar 3, 2023

Choose a reason for hiding this comment

videlec Mar 3, 2023

Choose a reason for hiding this comment

videlec left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 4, 2023

dcoudert commented Mar 4, 2023

videlec commented Mar 5, 2023 • edited Loading

dcoudert commented Mar 5, 2023

videlec commented Mar 5, 2023

dcoudert commented Mar 5, 2023

videlec commented Mar 5, 2023

PR for issue 32136: Add parameter 'induced' to `connected_subgraphs_iterator` #35131

PR for issue 32136: Add parameter 'induced' to `connected_subgraphs_iterator` #35131

dcoudert commented Feb 14, 2023 •

edited

Loading

codecov-commenter commented Feb 14, 2023 •

edited

Loading

videlec commented Mar 5, 2023 •

edited

Loading