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

Fix all_node_cuts output for complete graphs #6558

Merged
merged 3 commits into from Dec 22, 2023

Conversation

navyagarwal
Copy link
Contributor

Closes #6533

I modified the special condition for complete graphs in all_node_cuts to return an empty generator and also changed the test cases accordingly.

@navyagarwal navyagarwal changed the title All node cuts bug fix Fix all_node_cuts output for complete graphs Mar 26, 2023
@dschult
Copy link
Member

dschult commented Mar 29, 2023

Great! This should be merged after v3.2 is released and included in the release (along with removal of the deprecation warnings and conftest.py entires) of v3.3.

@dschult dschult added this to the networkx-3.2 milestone Mar 29, 2023
@jarrodmillman jarrodmillman removed this from the 3.2 milestone Jul 20, 2023
@MridulS
Copy link
Member

MridulS commented Dec 13, 2023

@navyagarwal if you still have the bandwidth could you add the deprecation warnings so we can push this as part of v3.3. Thanks!

@MridulS MridulS added this to the 3.3 milestone Dec 13, 2023
@dschult
Copy link
Member

dschult commented Dec 20, 2023

Based on my comments in #6533 I think there is more needed here.
And I'm not seeing now what would be deprecated.
The main problem is:

G=nx.complete_graph(5)
G.remove_edges_from([(0, 1), (3, 4)])
print(list(nx.all_node_cuts(G)))  # returns [{0, 1, 2}]

returns [{0, 1, 2}] when it should also include {2, 3, 4}.
Similar looking case works correctly:

G = nx.complete_graph(5)
G.remove_edges_from([(1, 2), (0, 4)])
print(list(nx.all_node_cuts(G))) # returns [{1, 2, 3}, {0, 3, 4}]

@dschult
Copy link
Member

dschult commented Dec 20, 2023

Yikes -- that took a bit. But I think I found an error in nx.all_node_cuts. Luckily it wasn't in the heart of the algorithm, but in the setup for which nodes get considered. A change from removing the potential node_cut set X to removing the currently considered node in that set {x} did the trick. I was following along with the article and it says "if x_i != v_j and v_j is not adjacent to x_i". It does not say that v_j should not be in the set X.

Anyway, it now works! With the fix already here for complete graphs, we have solved all the issues raised in #6533.

Ready for Review

@dschult dschult requested a review from MridulS December 22, 2023 08:05
@MridulS MridulS merged commit 1168afc into networkx:main Dec 22, 2023
39 checks passed
@MridulS
Copy link
Member

MridulS commented Dec 22, 2023

thanks @dschult @navyagarwal !

cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
* Fix for complete graphs case in all_node_cuts

* Check node_connectivty in complete graph

* test for broken cuts. add fix

---------

Co-authored-by: Dan Schult <dschult@colgate.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

all_node_cuts returns incorrect cuts
4 participants