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

Adding new feature wire_options to draw_mpl function #6486

Merged
merged 51 commits into from
Nov 8, 2024

Conversation

zazabap
Copy link
Contributor

@zazabap zazabap commented Oct 30, 2024

Context:
On the level of mid-scale algorithms, it might be nice to differentiate between different "types" of wires, for example by coloring them differently, or giving them distinct line styles. As an example, in PennyLaneAI/qml#1185 on preparing matrix product states, there are auxiliary bond qubits and physical qubits, and coloring them differently would be a neat thing to do.

Description of the Change:
Update the output wire_options that could change the line style and color for circuit output.

        @qml.qnode(qml.device("default.qubit"))
        def node(x):
            for w in range(5):
                qml.Hadamard(w) 
            return qml.expval(qml.PauliZ(0) @ qml.PauliY(1))

        # Make all wires cyan and bold, 
        # except for wires 2 and 6, which are dashed and another color
        wire_options = {"color": "cyan", 
                        "linewidth": 5, 
                        2: {"linestyle": "--", "color": "red"}, 
                        6: {"linestyle": "--", "color": "orange"}
                    }
        _,ax  = qml.draw_mpl(node, wire_options=wire_options)(0.52)

Benefits:
When complicated sates and quantum circuits diagram are created, wires could be marked with different selections.

Possible Drawbacks:
N/A

Related GitHub Issues:
#6165

@zazabap zazabap marked this pull request as draft October 30, 2024 16:02
@zazabap zazabap changed the title Starting a new PR and Add TODO Adding new feature wire_options to draw_mpl function Oct 30, 2024
@zazabap
Copy link
Contributor Author

zazabap commented Nov 1, 2024

Dear @dwierichs, I made necessary changes here for the test cases, documentation, and source code. However, I am not so familiar with the format check here. Sorry maybe I need some additional help on it.

@dwierichs
Copy link
Contributor

Hi @zazabap,
Great, thank you for your contribution. Is it ready to be reviewed? :)
Regarding the continuous integration checks, including formatting checks, take a look at our guide on how to submit a PR.
In particular, you want to pip install -r requirements-dev.txt while in the PennyLane directory and then run
black -l 100 pennylane tests. Other formatting tests likely will not be super relevant in this PR (unless pylint complains, but this we can handle once we get there :) )
Hope this helps! Let me know when we can move to review!

@zazabap
Copy link
Contributor Author

zazabap commented Nov 2, 2024

Thanks a lot for the help! I have reformatted the modified files and commit again, I think now it is ready for review. I resolved all the format check and unit test today @dwierichs .

@zazabap zazabap marked this pull request as ready for review November 2, 2024 05:19
Copy link

codecov bot commented Nov 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.34%. Comparing base (403c7a3) to head (39e1369).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6486   +/-   ##
=======================================
  Coverage   99.34%   99.34%           
=======================================
  Files         455      455           
  Lines       43193    43232   +39     
=======================================
+ Hits        42909    42948   +39     
  Misses        284      284           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Amazing addition @zazabap !
Looking forward to getting this feature in! 💯
I had some comments, in particular the documentation update should be moved to the docstrings of the respective functions, and the criterion with which we determine whether something is a per-wire option needs to be modified.

Thanks again, and let me know any questions you might have!

doc/code/qml_drawer.rst Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/code/qml_drawer.rst Outdated Show resolved Hide resolved
doc/code/qml_drawer.rst Outdated Show resolved Hide resolved
pennylane/drawer/mpldrawer.py Outdated Show resolved Hide resolved
pennylane/drawer/mpldrawer.py Outdated Show resolved Hide resolved
pennylane/drawer/mpldrawer.py Outdated Show resolved Hide resolved
tests/drawer/test_draw_mpl.py Show resolved Hide resolved
tests/drawer/test_draw_mpl.py Outdated Show resolved Hide resolved
@zazabap
Copy link
Contributor Author

zazabap commented Nov 5, 2024

Dear @dwierichs , I think now everything is updated and ready to merge. Please let me know if there are further questions or requests.

Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

@zazabap Thanks for the stamina with this contribution! 🎉
I will request a second review internally and then we can go ahead and merge :)

@astralcai astralcai self-requested a review November 5, 2024 20:28
Copy link
Contributor

@astralcai astralcai left a comment

Choose a reason for hiding this comment

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

Some minor comments

pennylane/drawer/mpldrawer.py Outdated Show resolved Hide resolved
pennylane/drawer/draw.py Outdated Show resolved Hide resolved
pennylane/drawer/tape_mpl.py Outdated Show resolved Hide resolved
@dwierichs
Copy link
Contributor

Hi @zazabap
We're experiencing problems with CI across the board. I think this PR is essentially merge-ready, so we'll merge it in as soon as we technically can :)
Thank you for this nice contribution! 🎉

Copy link
Contributor

@astralcai astralcai left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

@dwierichs dwierichs enabled auto-merge (squash) November 6, 2024 15:32
@dwierichs dwierichs added the merge-ready ✔️ All tests pass and the PR is ready to be merged. label Nov 8, 2024
@albi3ro albi3ro merged commit 73a617a into PennyLaneAI:master Nov 8, 2024
42 of 43 checks passed
mudit2812 pushed a commit that referenced this pull request Nov 11, 2024
**Context:**
On the level of mid-scale algorithms, it might be nice to differentiate
between different "types" of wires, for example by coloring them
differently, or giving them distinct line styles. As an example, in
PennyLaneAI/qml#1185 on preparing matrix product
states, there are auxiliary bond qubits and physical qubits, and
coloring them differently would be a neat thing to do.

**Description of the Change:**
Update the output wire_options that could change the line style and
color for circuit output.
```python
        @qml.qnode(qml.device("default.qubit"))
        def node(x):
            for w in range(5):
                qml.Hadamard(w) 
            return qml.expval(qml.PauliZ(0) @ qml.PauliY(1))

        # Make all wires cyan and bold, 
        # except for wires 2 and 6, which are dashed and another color
        wire_options = {"color": "cyan", 
                        "linewidth": 5, 
                        2: {"linestyle": "--", "color": "red"}, 
                        6: {"linestyle": "--", "color": "orange"}
                    }
        _,ax  = qml.draw_mpl(node, wire_options=wire_options)(0.52)
```

**Benefits:**
When complicated sates and quantum circuits diagram are created, wires
could be marked with different selections.

**Possible Drawbacks:**
N/A

**Related GitHub Issues:**
#6165

---------

Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: ringo-but-quantum <>
Co-authored-by: Astral Cai <[email protected]>
Co-authored-by: Christina Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-ready ✔️ All tests pass and the PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants