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

Bugfix: Lightning simulators do not support rotations anymore. #5343

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

vincentmr
Copy link
Contributor

@vincentmr vincentmr commented Mar 7, 2024

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    test directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the doc/releases/changelog-dev.md file, summarizing the
    change, and including a link back to the PR.

  • The PennyLane source code conforms to
    PEP8 standards.
    We check all of our code against Pylint.
    To lint modified files, simply pip install pylint, and then
    run pylint pennylane/path/to/file.py.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context:
Lightning simulators have never or currently do not support the optional rotations argument in their apply method. Sampling measurements are therefore performed in the wrong basis, among other issues.

Description of the Change:
Apply the diagonalizing gates within the sampling measurement section if the device is a Lightning device.
Apply the adjoint of the diagonalizing gates to restore the state for further processing.

Benefits:

Possible Drawbacks:

Related GitHub Issues:

@vincentmr vincentmr force-pushed the bugfix/lightning_rotations branch 2 times, most recently from f8c80bf to 1df7e57 Compare March 8, 2024 13:15
@vincentmr vincentmr changed the title Bugfix/lightning rotations Bugfix: Lightning simulators do not support rotations anymore. Mar 8, 2024
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (v0.35.1-rc@47b0394). Click here to learn what that means.

Additional details and impacted files
@@              Coverage Diff              @@
##             v0.35.1-rc    #5343   +/-   ##
=============================================
  Coverage              ?   99.64%           
=============================================
  Files                 ?      399           
  Lines                 ?    36628           
  Branches              ?        0           
=============================================
  Hits                  ?    36497           
  Misses                ?      131           
  Partials              ?        0           

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

@vincentmr vincentmr marked this pull request as ready for review March 8, 2024 16:09
@vincentmr vincentmr changed the base branch from master to v0.35.1-rc March 8, 2024 16:41
@vincentmr vincentmr requested a review from a team as a code owner March 8, 2024 16:41
@vincentmr vincentmr changed the base branch from v0.35.1-rc to master March 8, 2024 16:41
@vincentmr vincentmr changed the base branch from master to v0.35.1-rc March 8, 2024 16:45
Copy link
Contributor

@Mandrenkov Mandrenkov left a comment

Choose a reason for hiding this comment

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

I'm with @trbromley in that having at least one test to show this works would be nice!

pennylane/_qubit_device.py Outdated Show resolved Hide resolved
@Alex-Preciado Alex-Preciado removed the request for review from a team March 8, 2024 19:40
Mandrenkov added a commit that referenced this pull request Mar 8, 2024
**Context:**

There are two compatibility issues with the v0.35.0 release of PennyLane:
1. The minimum required Catalyst version is now v0.5.0 (#5320).
2. Lightning simulators need special handling of diagonalizing gates when performing sampling measurements (#5343).

This PR cherry-picks the squashed commit corresponding to (1) into the RC branch as the change necessary for (2) is already being made directly to the RC branch.

**Description of the Change:**

* Bumped the PennyLane Python package version to `v0.35.1`.
* Cherry-picked the following commits into the RC branch: a4f0fff and 79ce676.

**Benefits:**

* The latest PennyLane release works with the latest releases of Catalyst and Lightning.

**Possible Drawbacks:**

None.

**Related GitHub Issues:**

None
@vincentmr
Copy link
Contributor Author

I'm with @trbromley in that having at least one test to show this works would be nice!

I added a test that should pass and provide coverage.

@Mandrenkov
Copy link
Contributor

Mandrenkov commented Mar 8, 2024

I added a test that should pass and provide coverage.

Hey @vincentmr, I copied this test to a local version of the RC branch and it passed without these changes:

$ python -m pytest tests/devices/test_lightning_qubit.py -k "test_finite_shots"
=========================================================================================================== test session starts ============================================================================================================
platform darwin -- Python 3.9.13, pytest-8.0.2, pluggy-1.4.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /Users/mikhail/sc/58357/pennylane/tests
configfile: pytest.ini
plugins: flaky-3.7.0, cov-4.1.0, mock-3.12.0, xdist-3.5.0, benchmark-4.0.0, forked-1.6.0
collected 18 items / 16 deselected / 2 selected

tests/devices/test_lightning_qubit.py ..                                                                                                                                                                                             [100%]
===Flaky Test Report===

test_finite_shots passed 1 out of the required 1 times. Success!

===End Flaky Test Report===

===================================================================================================== 2 passed, 16 deselected in 0.10s =====================================================================================================

Are you sure we're testing the right thing here?

Edit: This was done using Lightning v0.35.0.

@vincentmr
Copy link
Contributor Author

Are you sure we're testing the right thing here?

Yes and no. Like I mentioned here, I wrote a test that passes with 0.35 and covers the lines, making sure nothing awfully wrong is going on. But the exhaustive tests and related fixes are going to be in the Lightning repo.

@vincentmr vincentmr merged commit b7080cc into v0.35.1-rc Mar 8, 2024
39 checks passed
@vincentmr vincentmr deleted the bugfix/lightning_rotations branch March 8, 2024 22:12
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.

None yet

4 participants