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

Implementation of post-selection for mid-circuit measurements #491

Merged
merged 13 commits into from
Feb 9, 2024

Conversation

rauletorresc
Copy link
Contributor

@rauletorresc rauletorresc commented Feb 2, 2024

Context: At the python level the postselect argument is optional and defaults to None. When given, it can only be 1 or 0. At the Quantum dialect level postselect is also optional, but when lowered to the LLVM dialect, the call to the measure function expect a mandatory postselect argument that can contain one of the following values:

  • -1, meaning postselect was not used
  • 0, for postselect = 0
  • 1, for postselect = 1

Related GitHub Issues: #448

TODO:

  • Complete the frontend implementation and connect it with the MLIR part.
  • Improve test at ConversionTests.mlir
  • Implement the post-selection logic at the runtime side.
  • ... also in Lightning.Kokkos
  • Add test cases.

[sc-46409]

@rauletorresc rauletorresc linked an issue Feb 2, 2024 that may be closed by this pull request
@rauletorresc rauletorresc self-assigned this Feb 2, 2024
@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from ca19450 to a15e879 Compare February 2, 2024 01:32
@rauletorresc rauletorresc added runtime Pull requests that update the runtime compiler Pull requests that update the compiler frontend Pull requests that update the frontend labels Feb 2, 2024
Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Nice work @rauletorresc! 🎸

frontend/catalyst/pennylane_extensions.py Outdated Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Outdated Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Show resolved Hide resolved
mlir/include/Quantum/IR/QuantumOps.td Outdated Show resolved Hide resolved
@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from a15e879 to 3e3cf85 Compare February 2, 2024 16:12
@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from 3e3cf85 to fd7543f Compare February 2, 2024 16:30
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

Nice job! Some suggestions and could you add a end to end integration test?

frontend/catalyst/pennylane_extensions.py Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Show resolved Hide resolved
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

Nice job, some comments left. I think there are still some issues in the runtime to be solved, let me know if you need some help

mlir/include/Quantum/IR/QuantumOps.td Outdated Show resolved Hide resolved
@rauletorresc
Copy link
Contributor Author

Still pending: Implement the post-selection logic at the runtime side.

@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from b1f66e3 to 54d6d1b Compare February 6, 2024 20:32
@rauletorresc rauletorresc marked this pull request as ready for review February 6, 2024 20:36
@rauletorresc
Copy link
Contributor Author

rauletorresc commented Feb 6, 2024

My only concern here is: all the shots are set up at the postselect value, e,g.: applying a Hadamard and having postselect=0, shots=10, we get the 10 shots as: [0,0,0,0,0,0,0,0,0,0]. Is that the intended behavior? I was hoiping to see [0,0,0,0,0]

Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Great work! 💯

@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from 54d6d1b to d3519f8 Compare February 8, 2024 03:52
@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch from d3519f8 to 466489e Compare February 8, 2024 16:52
Copy link
Contributor

@dime10 dime10 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 💯

mlir/include/Quantum/IR/QuantumOps.td Outdated Show resolved Hide resolved
runtime/include/QuantumDevice.hpp Show resolved Hide resolved
@rauletorresc rauletorresc force-pushed the implement-mid-circuit-post-selection branch 3 times, most recently from e87962c to 648f03e Compare February 9, 2024 00:57
@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (caf24e7) 99.57% compared to head (648f03e) 99.54%.

Files Patch % Lines
frontend/catalyst/pennylane_extensions.py 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #491      +/-   ##
==========================================
- Coverage   99.57%   99.54%   -0.03%     
==========================================
  Files          43       43              
  Lines        7740     7775      +35     
  Branches      533      536       +3     
==========================================
+ Hits         7707     7740      +33     
- Misses         17       18       +1     
- Partials       16       17       +1     

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

Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

🚀

doc/changelog.md Outdated Show resolved Hide resolved
runtime/include/QuantumDevice.hpp Outdated Show resolved Hide resolved
runtime/tests/Test_LightningMeasures.cpp Show resolved Hide resolved
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

Thanks for the PR it looks great!

doc/changelog.md Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Show resolved Hide resolved
Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Thanks @rauletorresc! 💯

@rauletorresc rauletorresc merged commit 0509f9e into main Feb 9, 2024
42 checks passed
@rauletorresc rauletorresc deleted the implement-mid-circuit-post-selection branch February 9, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Pull requests that update the compiler frontend Pull requests that update the frontend runtime Pull requests that update the runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement post-selection for mid-circuit measurements
7 participants