search

PennyLaneAI / pennylane-qiskit

The PennyLane-Qiskit plugin integrates the Qiskit quantum computing framework and IBM Q with PennyLane.
https://docs.pennylane.ai/projects/qiskit
Apache License 2.0
184 stars 66 forks source link

Ensure results when use_primitives = False vs use_primitives=True are consistent. #530

Closed austingmhuang closed 4 months ago

austingmhuang commented 4 months ago

To explain this change, some context behind the bug is necessary. We were seeing a difference between running qml.sample() when use_primitives was set to True vs False. When use_primitives was set to True, we were getting a shape of (1, 1024) instead of the correct shape of (1024, ). At first glance it seems like this problem should be resolved in execute -> you can simply check the len of the result that's returned there and if it's 1, take the 0th element instead. Although obvious, surprisingly, doing this does not fix the bug.

The root of the problem is actually in this split_execution_types function.

When running use_primitives = False, you skip split_execution_types, and therefore do not run this line of code. However, suppose result = [1]. Whenuse_primitives = True and len(result) == 1, you will be going from[1] -> (1,), which is bad since pennylane devices will return 1 instead. This is the source of the bug and it only occurs when a user sets use_primitives to false and has a circuit that only returns 1 measurement. (In other words, if the user had a circuit that returned multiple measurements, this bug would not occur).

austingmhuang commented 4 months ago

[sc-62867]

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (5e11e3a) to head (4ba38c2).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## new_device_feature_branch #530 +/- ## =========================================================== Coverage 100.00% 100.00% =========================================================== Files 8 8 Lines 848 849 +1 =========================================================== + Hits 848 849 +1 ```

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

austingmhuang commented 4 months ago

With use_primitives being removed as a keyword and the device upgrading to PrimitivesV2 as a whole, this PR will likely be significantly affected by those changes. I am going to leave this PR open for now but this will be blocked until we remove all the non-Qiskit 1.0/V2Primitives functionality.

austingmhuang commented 4 months ago

Closing this since the changes are now in #539