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 random_layout function and Pos2DMapping #305

Merged
merged 9 commits into from
Apr 19, 2021

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Apr 7, 2021

This commit adds a new function random_layout() which is used to
generate a random layout for a graph that can be used in visualization.
This is necessary for building a matplotlib drawer (issue #298 and a
first draft of the implementation #304). To make the function more
efficient it also adds a new custom return type Pos2DMapping which is
used to build an immutable readonly dict compatible result container for
the output type from this function.

Related to #280

@mtreinish mtreinish requested a review from georgios-ts April 7, 2021 12:29
@mtreinish mtreinish mentioned this pull request Apr 7, 2021
10 tasks
@mtreinish
Copy link
Member Author

There is a potential merge conflict here with #287 since the Pos2DMapping type borrows a lot of the implementation from that PR. We probably should merge #287 first and then rebase this on top.

This commit adds a new function random_layout() which is used to
generate a random layout for a graph that can be used in visualization.
This is necessary for building a matplotlib drawer (issue Qiskit#298 and a
first draft of the implementation Qiskit#304). To make the function more
efficient it also adds a new custom return type Pos2DMapping which is
used to build an imutable readonly dict compatible result container for
the output type from this function.

Related to Qiskit#280
@coveralls
Copy link

coveralls commented Apr 7, 2021

Pull Request Test Coverage Report for Build 763754537

  • 200 of 207 (96.62%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 96.152%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/iterators.rs 155 162 95.68%
Totals Coverage Status
Change from base Build 748553606: 0.02%
Covered Lines: 6347
Relevant Lines: 6601

💛 - Coveralls

Co-authored-by: Jielun (Chris) Chen <[email protected]>
Copy link
Contributor

@nahumsa nahumsa left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -154,3 +165,4 @@ Return Iterator Types
retworkx.NodeIndices
retworkx.EdgeList
retworkx.WeightedEdgeList
retworkx.Pos2DMapping
Copy link
Collaborator

Choose a reason for hiding this comment

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

with retworkx.*_layout(graph).keys() a user will see Pos2DMappingKeys. similar with.values(). Should we have docs for these types?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if it matters too much, we can add it to the documentation it doesn't hurt. I just left it out since there really isn't any documentation for those types and we don't actually export them in the retworkx module. I basically modeled these types on how python's dict works it 'returns a dict_keys, dict_values, and dict_items class for .keys(), .values(), and .items(). I couldn't find any documentation on those classes except in the C-API: https://docs.python.org/3/c-api/dict.html?highlight=dict_keys#c.PyDict_Keys

mtreinish and others added 3 commits April 19, 2021 09:35
This commit changes the input type of the center option to be a fixed 2
element array instead of a tuple. This still works as a tuple for the
python interface but has a defined contiguous memory layout in rust. It
also is consistent with the type for elements in the position mapping.
@mtreinish mtreinish merged commit 693faeb into Qiskit:main Apr 19, 2021
@mtreinish mtreinish deleted the random_layout branch April 19, 2021 19:45
@mtreinish mtreinish mentioned this pull request Apr 19, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants