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

expval ignores wire labels and uses position implicitly for tensor product ops #2276

Merged
merged 87 commits into from
Mar 18, 2022

Conversation

Jaybsoni
Copy link
Contributor

@Jaybsoni Jaybsoni commented Mar 3, 2022

Context:
Consider the following circuit:

dev = qml.device("default.qubit", wires=3)

@qml.qnode(dev)
def circ():
    qml.Hadamard(wires=0)
    qml.CNOT(wires=[0, 2])
    return qml.expval(qml.Identity(wires=0) @ qml.PauliZ(wires=1))      # -->  0.9999999999999998
    # return qml.expval(qml.Identity(wires=1) @ qml.PauliZ(wires=0))   # -->  0.9999999999999998
    # return qml.expval(qml.PauliZ(wires=0) @ qml.Identity(wires=1))   # -->  0.0
    # return qml.expval(qml.PauliZ(wires=1) @ qml.Identity(wires=0))   # -->  0.0

But the expected behaviour should be:

... 
    return qml.expval(qml.Identity(wires=0) @ qml.PauliZ(wires=1))      # -->  0.9999999999999998
    # return qml.expval(qml.Identity(wires=1) @ qml.PauliZ(wires=0))   # -->  0.0
    # return qml.expval(qml.PauliZ(wires=0) @ qml.Identity(wires=1))   # -->  0.0
    # return qml.expval(qml.PauliZ(wires=1) @ qml.Identity(wires=0))   # -->  0.9999999999999998

It seems as though the wire labels are being ignored and instead the position of the operations in the tensor product are being used to determine how to compute the expectation value.

Additionally:
This PR also addresses the following bug which was discovered during the testing of this feature:

    dev = qml.device("default.qubit", wires=3)
    theta = 0.432
    phi = 0.123
    varphi = -0.543
 
    @qml.qnode(dev)
    def circuit(o):
        qfunc()
        return qml.expval(qml.apply(o))

    obs0 = qml.PauliZ(wires=2)

    obs1 = qml.Identity(wires=0) @ qml.Identity(wires=1) @ qml.PauliZ(wires=2)
    obs2 = qml.Identity(wires=1) @ qml.Identity(wires=0) @ qml.PauliZ(wires=2)

    obs3 = qml.Identity(wires=0) @ qml.PauliZ(wires=2) @ qml.Identity(wires=1)
    obs4 = qml.Identity(wires=1) @ qml.PauliZ(wires=2) @ qml.Identity(wires=0)

    obs5 = qml.PauliZ(wires=2) @ qml.Identity(wires=0) @ qml.Identity(wires=1)
    obs6 = qml.PauliZ(wires=2) @ qml.Identity(wires=1) @ qml.Identity(wires=0)

    obs_lst = [obs0, obs1, obs2, obs3, obs4, obs5, obs6]

    for i, o in enumerate(obs_lst):
        print(f"------ {i} ------- \n{circuit(o)}")

Which produces the following result:

------ 0 -------
0.8561624160163042
------ 1 -------
0.8561624160163042
------ 2 -------
0.8561624160163042
------ 3 -------
0.8561624160163042
------ 4 -------
0.9081301906949608
------ 5 -------
0.9924450321351934
------ 6 -------
0.8561624160163042

In the above result we notice that the expectation values computed for observables 4, 5 do not match the others.

Description of the Change:
The expval(self, observable, shot_range=None, bin_size=None) function defined in _qubit_device.py uses the eigenvalues of the provided tensor product observable and the probability vector of the state to compute the expectation value when shots=None.

The first bug arises because of lexicographic sorting of the eigvals based on the wires the observables act on in the function qml.Tensor.get_eigvals(). This results in the eigvals of the two observables PauliZ @ Identity and Identity @ PauliZ being the same.

This is addressed by removing the sorting in the get_eigvals() function.

The second bug arises because of the way we permute the probability vector before computing the expectation value. The wire order of the observables is passed into the device.probability() function which then computes the marginal probability distribution over the observable wires and permutes the distribution according to the wire order of the observable wires. This permutation is incorrect and instead we should be using the inverse of this permutation. The reason such a bug was not caught sooner is because in most cases the tensor product observables usually contain 2 terms at most and/or there is only one qubit flipped out of position. In these common cases, the wire order permutation is its own inverse, and thus produces the correct expected value. But in situations with more then one qubit flip, it breaks.

This is addressed by computing the permutation before hand, then permuting the wires before they are passed into the probability function.

Related GitHub Issues:
This PR closes this issue

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

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.

@Jaybsoni
Copy link
Contributor Author

Jaybsoni commented Mar 3, 2022

[sc-15486]

@Jaybsoni
Copy link
Contributor Author

Jaybsoni commented Mar 3, 2022

The reason some of the tests are failing is because of the following edge case:

A = np.array( ... some hermitian 8x8 matrix ... )
H = qml.Hermitian(A, wires=[2,1,0])

def circ(): 
... some ops ...
    return qml.expval(H)   # now that we sort the wires for the probability vector but not for the
                           # Eigen_values, we get the "wrong" answer! 

The answer would be "correct" if we interpreted the matrix as already being transformed under the provided wire order.

Jaybsoni and others added 7 commits March 4, 2022 10:17
* Exclude Snapshot from adjoint backwards pass

* Add snapshots test for diff_methods

* Changelog

* Trigger rebuild

Co-authored-by: antalszava <antalszava@gmail.com>
* some inconsistencies

* swap basis

* undo duplicated wire in test

* changelog

* revert snapshot wires change

* unused import

* merge rc

* revert accidental changelog merge

Co-authored-by: Josh Izaac <josh146@gmail.com>
* batching ability for non-trainable inputs only following issue #2037

This function creates multiple circuit executions for batched input examples
and executes all batch inputs with the same trainable variables. The main
difference between the proposed version in the issue and this commit is the
input `argnum` this indicates the location of the given input hence gives the
ability to work across platforms.

* adaptation for batch execution

* improvements according to PR rules

* minor update according to PR errors

* modify according to codefactor-io

* reformatted code style

* adjust line lenght for linting

* update linting

* disable linting for too many arguments

* add testing for batch input in keras

* format test_keras.py

* add tests for remaining functions

* adapt the defaults

* update docstring according to @josh146 's suggestions

* remove keras sterilazation

* add batch_input to the docstring

* docstring update for readability: pennylane/transforms/batch_input.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

* minor fix in documentation

* change assertion error to valueerror

* test valueerror

* modify the definition of argnum

* change argnum -> batch_idx

* update changelog-dev.md

* apply @josh146 's suggestions

* linting

* tests

* more

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
@antalszava antalszava changed the base branch from master to v0.22.0-rc0 March 8, 2022 20:27
antalszava and others added 12 commits March 8, 2022 15:27
* Redo imports

* Update docs

* Update wording

* Fix ID

* Add to docs

* Add to docs

* Fix

* Update docstrings

* Use nx.MultiDiGraph

* Update fragment_graph

* Update graph_to_tape

* Update remap_tape_wires

* Rename to expand_fragment_tape

* Update expand_fragment_tape

* Update CutStrategy

* Update qcut_processing_fn

* Remove note

* Update cut_circuit

* Work on docs

* Add to docs

* Update pennylane/transforms/qcut.py

* Add to changelog

* Move device definition

* Mention WireCut

* Move details

* QCut module

* Fix image location

* Fix init

* Apply suggestions from code review

Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>

* Add link to communication graph

* Reword

* Move around

* fix

* fix

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
* Redo imports

* Update docs

* Update wording

* Fix ID

* Add to docs

* Add to docs

* Fix

* Update docstrings

* Use nx.MultiDiGraph

* Update fragment_graph

* Update graph_to_tape

* Update remap_tape_wires

* Rename to expand_fragment_tape

* Update expand_fragment_tape

* Update CutStrategy

* Update qcut_processing_fn

* Remove note

* Update cut_circuit

* Work on docs

* Add to docs

* Update pennylane/transforms/qcut.py

* Add to changelog

* Move device definition

* Mention WireCut

* Move details

* QCut module

* Fix image location

* Fix init

* Update changelog

* Link to docs page

* Update wording

* Apply suggestions from code review

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Move

* Update doc/releases/changelog-dev.md

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Remove

* Update

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
* Produce consisten output shapes

In the absence of trainable params, some gradient transforms did not
produce an empty tuple yet like the rest of our functions.

* Minor formatting changes in param_shift_hessian

* Fix param_shift_hessian for all zero diff_methods

* Fix missing requires_grad & catch expected warning

* Changelog

Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
* Deprecate the Jacobian tape

* Deprecate tape subclasses

* changelog

* more test fixes

* tests

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>

Co-authored-by: antalszava <antalszava@gmail.com>
* generator doc fixes

* more fixing
* Remove temp fixes for lightning

* Include diff_method tests for all devices

* Changelog

* Update CI to use pennylane-lightning dev

Co-authored-by: antalszava <antalszava@gmail.com>
* Fix Operator docstring hyperrefs

* Fix example for top-level matrix function

* Add example to Snapshot op docstring

* Fix tape drawing examples in docs

* Apply suggestions from code review

* Update pennylane/ops/snapshot.py

Co-authored-by: Christina Lee <christina@xanadu.ai>
* Add qfunc and else to cond's UsageDetails

* copy when inverting MV under the hood; add equivalent test case for inversion; add err msg when calling == of MV with unexpected typed obj; more examples

* format

* test docstr

* format

* correct examples

* format

* docstring

* have #2300 on rc too

* lambda example

* intro extend, docstring

* changelog PR num

* link

* note update

* updates

* Apply suggestions from code review

* updates

Co-authored-by: Christina Lee <christina@xanadu.ai>
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 🥇 Amazing solution, goes a long way with understanding what's happening here.

Codefactor seems to cause some issues still and left come comments, but once those are addressed, this is good to be merged from my side.

pennylane/_device.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.py Outdated Show resolved Hide resolved
pennylane/devices/tests/test_measurements.py Outdated Show resolved Hide resolved
pennylane/devices/tests/test_measurements.py Outdated Show resolved Hide resolved
pennylane/devices/tests/test_measurements.py Outdated Show resolved Hide resolved
pennylane/devices/tests/test_measurements.py Show resolved Hide resolved
pennylane/devices/tests/test_measurements.py Outdated 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.

9 participants