search

PennyLaneAI / pennylane-cirq

The PennyLane-Cirq plugin integrates Google's Cirq software library with with PennyLane's quantum machine learning capabilities.
https://docs.pennylane.ai/projects/cirq
Apache License 2.0
50 stars 18 forks source link

remove expval ovverride; allow atol for cirq mixed state simulator test #150

Closed timmysilv closed 1 year ago

timmysilv commented 1 year ago

With numpy <1.25, the probs are identical ([0.49999967, 0.49999967]) before dotting with the eigenvalues so the result is exactly zero. With numpy >=1.25, they are not ([0.49999964, 0.49999958]). This slight change is very mysterious to me, but I have confirmed that it appears at this line. The inputted self.circuit is identical, and the outputted result is reliably off. Each value in the state is off by up to 3e-8, and I don't know why (they aren't totally random rounding errors because the difference between old and new states are numbers like 1.00000e-8 or 9.3400000e-9).

My solution for now is to allow an atol of 5e-8.

EDIT: I've removed the expval override since it's not covered anymore. It was needed with the old numpy - should I pin the minimum, or keep the expval and put a no-cover comment on it?

EDIT 2: I decided to keep the override so we can still use old numpy without issues, and I added a test that forces use of the old one for codecov happiness

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (8447c11) 99.14% compared to head (e87a73d) 99.14%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #150 +/- ## ======================================= Coverage 99.14% 99.14% ======================================= Files 8 8 Lines 352 352 ======================================= Hits 349 349 Misses 3 3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

timmysilv commented 1 year ago

@mudit2812 Christina is looking into that in another PR

codecov is failing because of a fallback mechanism that should probably be stated as no-cover, and perhaps will be hit less frequently with this numpy bump? I saw the issue locally in both scenarios so I'm not sure what's going on...

more context: computing expval with a density matrix sometimes fails some validation in cirq code here and that raises a ValueError. In those cases, we default to QubitDevice expval. It seems like this test was the only one doing that, and it doesn't seem to default with the latest numpy.

timmysilv commented 1 year ago

updated the description with an EDIT, pls check it out