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 gnm_random_graph functions #167

Closed
mtreinish opened this issue Oct 6, 2020 · 7 comments
Closed

Add gnm_random_graph functions #167

mtreinish opened this issue Oct 6, 2020 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mtreinish
Copy link
Member

What is the expected enhancement?

While retworkx has a random G_n,p graph functions there are some use cases for a random G_n,m graphs. Specifically: https://github.com/Qiskit/qiskit-terra/blob/01ded73d1ded2342e39bc22d994ba0b50ffc8de7/test/python/transpiler/test_token_swapper.py#L105 We should add this to retworkx too. This could be part of #150 but opening as a a separate issue since it's a feature gap for terra (well terra's tests at least).

@mtreinish mtreinish added enhancement New feature or request good first issue Good for newcomers labels Oct 6, 2020
@MoAllabbad
Copy link
Contributor

Should this return an undirected graph? terra's usage doesn't seem to care.

@mtreinish
Copy link
Member Author

mtreinish commented Oct 16, 2020

This would be to add both an undirected and a directed function, for example similar to: https://github.com/Qiskit/retworkx/blob/master/src/lib.rs#L1452 and https://github.com/Qiskit/retworkx/blob/master/src/lib.rs#L1536

The terra use case I linked to just expects an undirected graph (that's the default type from networkx). But the work to implement the function is more or less identical so might as well add both.

@MoAllabbad
Copy link
Contributor

Sounds good! I'll start working on this if that's ok.

@mtreinish
Copy link
Member Author

Sure, of course. I just assigned the issue to you.

@MoAllabbad
Copy link
Contributor

Thanks. On a side note. The implementation of retwokx's gnp doesn't accept exact 0 and 1 probabilities, whereas networkx does, returning empty or full graphs respectively. I'm not sure of the significance of that.

@mtreinish
Copy link
Member Author

Thanks. On a side note. The implementation of retwokx's gnp doesn't accept exact 0 and 1 probabilities, whereas networkx does, returning empty or full graphs respectively. I'm not sure of the significance of that.

Oh, that's a fair point. It's just an oversight really. I'm not sure if there is a use case for it since you can just create an empty and full graph directly pretty easily, but it does seem like something worth adding. Feel free to open an issue or push a PR to fix it and we can go from there.

@MoAllabbad
Copy link
Contributor

Oh, that's a fair point. It's just an oversight really. I'm not sure if there is a use case for it since you can just create an empty and full graph directly pretty easily, but it does seem like something worth adding. Feel free to open an issue or push a PR to fix it and we can go from there.

Opened an issue #172 and should push a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants