Add margins for conv graph plot #993
Conversation
ryanhill1
left a comment
There was a problem hiding this comment.
Thanks for the PR! Your changes do seem to resolve the issue of nodes being cut off at the edges. However, reducing the margins has the side effect of compressing the nodes toward the center, which makes the graph slightly harder to read.
One way to address this, like you mentioned, could be by introducing a keyword argument to control the margin size—but that introduces a bit of a trade-off. Ideally, instead of shrinking the plot area, we’d find a way to prevent nodes from being plotted beyond a certain perimeter, preserving their current spacing while avoiding cutoff.
Would you be open to exploring that approach? If it's not feasible, we can go ahead with the margin parameter as a fallback.
|
Here are some examples that show how reducing the margins causes the nodes to appear more crowded in the center. While this does fix the original issue of nodes being cut off, it introduces a new issue with overly tight spacing, which could be seen as equally problematic. Example 1 before:
Example 1 after:
Example 2 before:
Example 2 after:
|
|
We've made "plot_margin" a keyword argument with a default value of 0.1 (10%). This seems to be a better value than the original 0.15 in terms of the use of space within the plot canvas. The rx.spring_layout doesn't appear to provide many controls by which the distribution of nodes can be modified. However, judicious use of the "repulsive_exponent" argument may improve the spacing and readability of the graph plot (repulsive_exponent is an integer with a default value of 2). Here are three examples, each using the new default value of the plot_margin argument (note that we have turned on the visible axes for the purposes of gauging the extent of the plot canvas): graph.plot(seed=362, legend=True) graph.plot(seed=362, legend=True, repulsive_exponent=0) graph.plot(seed=362, legend=True, repulsive_exponent=-1) |
ryanhill1
left a comment
There was a problem hiding this comment.
Ok, thanks for looking into that. Instead of using kwargs.pop can you explicitly add margin to the kwargs and docstring of the plot_conversion_graph function, and update CHANGELOG.md? After that, this should be good to go.
|
Thank you! I hope this closes the issue :) |
|
Please include the changelog update in this PR, not a new one. And link back to this PR, not the issue. |
|
I have made the changes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
* Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * format, re-word docstring, update arg name * update changelog for arg name consistency
…,<0.41 (#991) * Update qiskit-ibm-runtime requirement Updates the requirements on [qiskit-ibm-runtime](https://github.com/Qiskit/qiskit-ibm-runtime) to permit the latest version. - [Release notes](https://github.com/Qiskit/qiskit-ibm-runtime/releases) - [Commits](Qiskit/qiskit-ibm-runtime@0.25.0...0.40.0) --- updated-dependencies: - dependency-name: qiskit-ibm-runtime dependency-version: 0.40.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> * Put upper bound on `pydantic` version * Add margins for conv graph plot (#993) * Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update plot_conversions.py * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md * format, re-word docstring, update arg name * update changelog for arg name consistency * Reset CHANGELOG.md for new version [no ci] (#996) * Bump patch version to 0.9.7 (#997) Co-authored-by: qbraidTeam <[email protected]> * Update CHANGELOG.md --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ryan Hill <[email protected]> Co-authored-by: Ash <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: qbraidTeam <[email protected]>
Summary of changes
The autoscaling that matplotlib uses will sometimes cause the plot to be clipped. This is more likely to occur when using the "scatter" plot function with large symbols. This change does two things:
The margin around the plot allows the contents to be rendered without clipping.
Note that currently, the margin is hard coded at 15%. To make this more flexible it could be added as an extra keyword argument for the plot_conversion_graph function.
Potentially closes #851