8000 Fix sparse heuristic not applied to diagonal blocks in stochastic_block_model by dean985 · Pull Request #8557 · networkx/networkx · GitHub
[go: up one dir, main page]

Skip to content

Fix sparse heuristic not applied to diagonal blocks in stochastic_block_model#8557

Open
dean985 wants to merge 1 commit intonetworkx:mainfrom
dean985:fix-sbm-sparse-diagonal-blocks
Open

Fix sparse heuristic not applied to diagonal blocks in stochastic_block_model#8557
dean985 wants to merge 1 commit intonetworkx:mainfrom
dean985:fix-sbm-sparse-diagonal-blocks

Conversation

@dean985
Copy link
Contributor
@dean985 dean985 commented Mar 7, 2026

fixes #8536

Summary

  • The diagonal block (i==j) case in stochastic_block_model consumed the edges iterator before the sparse/dense branching logic, so the sparse heuristic was never used for diagonal blocks.
  • Removed the premature edge addition (lines 638-640) so diagonal blocks flow into the same sparse/dense logic as off-diagonal blocks.
  • Updated the expected edge count in the test to reflect the corrected behavior.

Test plan

  • All existing tests in test_community.py pass (22/22)
  • Updated test_stochastic_block_model expected edge count to match the corrected sparse heuristic behavior

…ck_model

The diagonal block (i==j) case consumed the edges iterator before the
sparse/dense branching logic, so the sparse heuristic was never used for
diagonal blocks. Remove the premature edge addition so diagonal blocks
flow into the same sparse/dense logic as off-diagonal blocks.
Copy link
Member
@Peiffap Peiffap left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor
@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Presumably the change in results is a result of the random state changing due to the removal of special handling of the i==j case. I think this is fine, though it's worth noting that this change will have an impact even outside of the sparse codepath as a result. Again this is probably fine, but just pointing it out so folks more familiar with the stochastic block model can take a look and double check!

Copy link
Contributor
@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Also - it'd be nice to have a test that illustrates the original issue in the sparse branch (presumably that the sparsity algorithm wasn't being applied to the diagonal blocks) and illustrates the fix.

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

Develop 413E ment

Successfully merging this pull request may close these issues.

Not using sparse algorithm in stochastic_block_model code

3 participants

0