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

Add stochastic block model generator #1200

Merged
merged 6 commits into from
May 25, 2024
Merged

Conversation

SILIZ4
Copy link
Contributor

@SILIZ4 SILIZ4 commented May 22, 2024

This naive O(n^2) algorithm might not be the fastest possible algorithm since there exists a O(m) algorithm for G(n, p), but I didn't see how to generalize it for non square "sub-matrices". I'm not convinced that networkx's implementation with iterators is actually more efficient.

I was unable to add hyperlinks to the G(n, p) generator in rustworkx and rustworkx-core. Is there a way to do so?

@coveralls
Copy link

coveralls commented May 22, 2024

Pull Request Test Coverage Report for Build 9238376817

Details

  • 132 of 133 (99.25%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 96.025%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rustworkx-core/src/generators/random_graph.rs 72 73 98.63%
Totals Coverage Status
Change from base Build 9192798441: 0.02%
Covered Lines: 17105
Relevant Lines: 17813

💛 - Coveralls

Copy link
Collaborator

@IvanIsCoding IvanIsCoding left a comment

Choose a reason for hiding this comment

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

I haven't had time to give a complete review but:

  • Complexity seems fine, if you think there are improvements you can open an issue after we merge this one
  • We should probably accept numpy arrays as an input, I have a feeling that is the datatype people would use for a matrix

/// Arguments:
///
/// :param list[int] blocks: Block membership (between 0 and B-1) of each node.
/// :param list[list[float]] probabilities: B x B matrix that contains the
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can probably be a 2d numpy.array

rustworkx-core/src/generators/random_graph.rs Outdated Show resolved Hide resolved
Comment on lines +7 to +9
Adds new function ``sbm_random_graph`` to the rustworkx-core module
``rustworkx_core::generators`` that samples a graph from the stochastic
block model.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add drawings of some example graphs if you want, check

:func:`.directed_barabasi_albert_graph` and :func:`.barabasi_albert_graph`,
for an example

@IvanIsCoding
Copy link
Collaborator

This naive O(n^2) algorithm might not be the fastest possible algorithm since there exists a O(m) algorithm for G(n, p), but I didn't see how to generalize it for non square "sub-matrices". I'm not convinced that networkx's implementation with iterators is actually more efficient.

I was unable to add hyperlinks to the G(n, p) generator in rustworkx and rustworkx-core. Is there a way to do so?

By the way, for links you want:

@SILIZ4
Copy link
Contributor Author

SILIZ4 commented May 25, 2024

I wasn't sure how dependencies versions should be handled so I added ndarray directly with cargo. Docs hyperlinks are fixed. I also changed the first argument for the community sizes instead of membership. It makes more sense and is what networkx uses.

@IvanIsCoding
Copy link
Collaborator

I wasn't sure how dependencies versions should be handled so I added ndarray directly with cargo. Docs hyperlinks are fixed. I also changed the first argument for the community sizes instead of membership. It makes more sense and is what networkx uses.

I will tweak ndarray to use Cargo workspaces

@IvanIsCoding IvanIsCoding added the automerge Queue a approved PR for merging label May 25, 2024
@IvanIsCoding IvanIsCoding merged commit f3b45f7 into Qiskit:main May 25, 2024
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Queue a approved PR for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants