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

Rework expectation values using Cirq simulators #81

Merged
merged 31 commits into from
Oct 1, 2021
Merged

Rework expectation values using Cirq simulators #81

merged 31 commits into from
Oct 1, 2021

Conversation

rmoyard
Copy link
Contributor

@rmoyard rmoyard commented Aug 25, 2021

This PR aims to update the way expval is evaluated, it can now use simulate_expectation_valuesfrom Cirq instead of the PennyLane function for cirq.simulator and cirq.mixedsimulator for shotsand shots=None. It is directly related to issue #70.

@rmoyard rmoyard added the WIP Work in Progress label Aug 25, 2021
@rmoyard rmoyard changed the title Rework expectation values [WIP] Rework expectation values Aug 25, 2021
@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #81 (d7d0e11) into master (fd7f84d) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
+ Coverage   99.14%   99.23%   +0.09%     
==========================================
  Files           8        8              
  Lines         349      391      +42     
==========================================
+ Hits          346      388      +42     
  Misses          3        3              
Impacted Files Coverage Δ
pennylane_cirq/cirq_device.py 98.92% <100.00%> (+0.02%) ⬆️
pennylane_cirq/qsim_device.py 96.96% <100.00%> (+0.41%) ⬆️
pennylane_cirq/simulator_device.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd7f84d...d7d0e11. Read the comment docs.

@rmoyard rmoyard requested a review from antalszava September 7, 2021 14:07
@rmoyard rmoyard changed the title [WIP] Rework expectation values Rework expectation values using Cirq simulators Sep 7, 2021
@antalszava antalszava requested review from thisac and removed request for antalszava September 8, 2021 11:34
Copy link
Contributor

@thisac thisac 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 @rmoyard. Great job getting this to work. 🚀

Just have a few comments/questions on a few things. Mainly code-quality things. Regarding the failing device suite test, I'm not sure what to do. It seems like something that might need to be fixed on the Cirq side. 🤔

pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
pennylane_cirq/simulator_device.py Show resolved Hide resolved
pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
pennylane_cirq/qsim_device.py Outdated Show resolved Hide resolved
@rmoyard rmoyard removed the WIP Work in Progress label Sep 9, 2021
rmoyard and others added 2 commits September 13, 2021 09:24
Co-authored-by: Theodor <theodor@xanadu.ai>
@antalszava antalszava self-requested a review September 20, 2021 14:28
Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Hi @rmoyard, this is looking nice altogether! 💯 🙂 Left some comments with suggestions. Most of these include thoughts on readability and testing.

pennylane_cirq/qsim_device.py Outdated Show resolved Hide resolved
pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
pennylane_cirq/simulator_device.py Show resolved Hide resolved
pennylane_cirq/simulator_device.py Show resolved Hide resolved
pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
tests/test_expval.py Outdated Show resolved Hide resolved
tests/test_mixed_simulator_device.py Outdated Show resolved Hide resolved
tests/test_qsim_device.py Outdated Show resolved Hide resolved
tests/test_qsim_device.py Outdated Show resolved Hide resolved
tests/test_qsim_device.py Outdated Show resolved Hide resolved
Copy link
Contributor

@antalszava antalszava 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! 💯 🎉 Awesome to have this in. 🙂 Had two minor comments, happy for this to be merged from my side afterwards.

pennylane_cirq/simulator_device.py Outdated Show resolved Hide resolved
tests/test_qsim_device.py Show resolved Hide resolved
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