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

introduce QubitDevice._get_diagonalizing_gates; skip Hamiltonian in default.qubit #3938

Merged
merged 8 commits into from
Mar 24, 2023

Conversation

timmysilv
Copy link
Contributor

@timmysilv timmysilv commented Mar 23, 2023

Context:
Can be done instead of #3929. Some devices compute diagonalizing gates, and this can be costly. If they aren't going to be used, a device should be able to skip trying.

Description of the Change:

  1. Add _get_diagonalizing_gates to the QubitDevice class. It is a copy-paste of the QuantumScript.diagonalizing_gates property.
  2. Replace circuit.diagonalizing_gates with self._get_diagonalizing_gates(circuit) in QubitDevice.execute (when it calls self.apply).
  3. Override the method in DefaultQubit to filter out any measurements with isinstance(m.obs, qml.Hamiltonian).

Benefits:

  • The device can weigh in on what rotation gates should be executed/evaluated for a circuit
  • We move away from QuantumTape.diagonalizing_gates, whose reason for existence is questionable

Possible Drawbacks:

  • More code in default.qubit that needs maintenance/porting over
  • Duplicate of QuantumScript.diagonalizing_gates code

Additional Notes

  • I put this in QubitDevice rather than Device to minimize changes to the outward-facing Device API, because I know the scope is limited. I can move it to Device if people prefer it, but this was an intentional choice
  • I prefixed the method name with get_ on purpose, to show that it's doing something (unlike the aforementioned property which was intentionally hiding that fact), and to distinguish it from existing code. Again, I can change this if people prefer it

Related GitHub Issues:
#3929, blocks PennyLaneAI/pennylane-lightning#424

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #3938 (59bb0ef) into master (c957db9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3938   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         345      345           
  Lines       30164    30171    +7     
=======================================
+ Hits        30086    30093    +7     
  Misses         78       78           
Impacted Files Coverage Δ
pennylane/_qubit_device.py 99.38% <100.00%> (+<0.01%) ⬆️
pennylane/devices/default_qubit.py 99.70% <100.00%> (+<0.01%) ⬆️
pennylane/devices/null_qubit.py 100.00% <100.00%> (ø)
pennylane/tape/qscript.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@timmysilv timmysilv changed the title introduce QubitDevice.get_diagonalizing_gates; skip Hamiltonian in default.qubit introduce QubitDevice._get_diagonalizing_gates; skip Hamiltonian in default.qubit Mar 23, 2023
@timmysilv timmysilv requested a review from a team March 23, 2023 19:48
pennylane/_qubit_device.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.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.

I'm quite happy with these changes. I like the shift of moving diagonalizing gates to the device from the qscript a lot. Thanks Matt!

Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Make sure to add a changelog!

@timmysilv timmysilv requested a review from albi3ro March 24, 2023 20:10
Copy link
Contributor

@albi3ro albi3ro 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 to me 👍

@timmysilv timmysilv enabled auto-merge (squash) March 24, 2023 20:43
@timmysilv timmysilv merged commit 41df820 into master Mar 24, 2023
@timmysilv timmysilv deleted the device-diagonalizing-gates branch March 24, 2023 21:04
mudit2812 pushed a commit that referenced this pull request Apr 13, 2023
…efault.qubit (#3938)

* introduce QubitDevice.get_diagonalizing_gates; skip Hamiltonian in default.qubit

* fix filter in default.qubit

* make method private; add tests

* default uses tape property; add type hints

* changelog entry

* trigger ci

* move to improvements in changelog
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.

3 participants