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

Draw qubit reset and postselection with matplotlib #4832

Merged
merged 20 commits into from
Nov 24, 2023
Merged

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Nov 13, 2023

Screenshot 2023-11-13 at 2 36 22 PM

This PR accomplishes two main things:

  • Indicates postselection on the icon for mid circuit measurements
  • Indicates reset with erasing the wire line and restarting it after a |0> box.

There is a regression in the handling of conditionals, as drawing conditionals will be standardized in a follow-up PR.

While doing so, I also:

  • converted from a hacky single dispatch to functools.singledispatch to register custom ways to display operations
  • Moved Conditional to the ops.op_math submodule to avoid a circular dependency. This can be accomplished in a separate PR instead.

[sc-50118]

@trbromley
Copy link
Contributor

Thanks @albi3ro, this looks great! One minor suggestion, which could be done in a separate PR: could the measurement symbol (semicircle and arrow) be moved downwards inside the box so that it is centre-aligned vertically?

@albi3ro
Copy link
Contributor Author

albi3ro commented Nov 14, 2023

@trbromley like this?

Screenshot 2023-11-14 at 10 11 10 AM

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8d1a94e) 99.65% compared to head (7cf7027) 99.64%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4832      +/-   ##
==========================================
- Coverage   99.65%   99.64%   -0.02%     
==========================================
  Files         383      383              
  Lines       34582    34341     -241     
==========================================
- Hits        34462    34218     -244     
- Misses        120      123       +3     

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

@albi3ro albi3ro marked this pull request as ready for review November 15, 2023 19:37
@albi3ro albi3ro requested a review from a team November 22, 2023 15:39
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Looks great Christina!

image

Is it possible to lower the number drawn for postselection? Imo it's too close to the measurement symbol. Other than than, I'm ready to approve.

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/drawer/mpldrawer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

The link to this PR still needs to be added to the changelog, but other than that looks good! Thanks Christina, the singledispatch was a really good change as well

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@timmysilv
Copy link
Contributor

what is the plan for conditionals? are we going to restore them to the current behaviour, or do something all-new? not entirely sure why it should be done in another PR, but i'm fine to keep things separated with a plan laid out

@albi3ro
Copy link
Contributor Author

albi3ro commented Nov 24, 2023

what is the plan for conditionals? are we going to restore them to the current behaviour, or do something all-new? not entirely sure why it should be done in another PR, but i'm fine to keep things separated with a plan laid out

@timmysilv PR #4850 . Depends on Mudit's PR #4803 .

Copy link
Contributor

@timmysilv timmysilv left a comment

Choose a reason for hiding this comment

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

looks good!

@albi3ro albi3ro enabled auto-merge (squash) November 24, 2023 16:29
@albi3ro albi3ro merged commit d13cf3d into master Nov 24, 2023
35 checks passed
@albi3ro albi3ro deleted the mpl-midmeasure branch November 24, 2023 16:42
albi3ro added a commit that referenced this pull request Dec 7, 2023
This PR depends on #4832 and #4803 



![midmeasure_mpl](https://github.com/PennyLaneAI/pennylane/assets/6364575/06147924-684d-4bd5-a8aa-5a266bf19852)

[sc-48302]

---------

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
mudit2812 added a commit that referenced this pull request Jan 19, 2024
This PR depends on #4832 and #4803 



![midmeasure_mpl](https://github.com/PennyLaneAI/pennylane/assets/6364575/06147924-684d-4bd5-a8aa-5a266bf19852)

[sc-48302]

---------

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
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.

4 participants