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

Create qml.PauliError #1781

Merged
merged 47 commits into from
Dec 6, 2021
Merged

Conversation

MoritzWillmann
Copy link
Contributor

@MoritzWillmann MoritzWillmann commented Oct 21, 2021

Context: Additional channel qml.ops.PauliError

Description of the Change: Adding error channel for arbitrary Pauli operations on arbitrary number of wires

Benefits: More flexible error channel

Possible Drawbacks: Possibly expensive, no nice interface

Related GitHub Issues: closes #1529

Note: a package called mistune used by m2r2 has been raising errors when running the documentation CI checks. This PR further closes #1983, to resolve this issue.

cc: @antalszava

@codecov
Copy link

codecov bot commented Oct 21, 2021

Codecov Report

Merging #1781 (71b2764) into master (06a15ff) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1781      +/-   ##
==========================================
+ Coverage   98.80%   98.81%   +0.01%     
==========================================
  Files         225      227       +2     
  Lines       17123    17291     +168     
==========================================
+ Hits        16918    17086     +168     
  Misses        205      205              
Impacted Files Coverage Δ
pennylane/_grad.py 100.00% <ø> (ø)
pennylane/devices/default_mixed.py 100.00% <ø> (ø)
pennylane/transforms/classical_jacobian.py 100.00% <ø> (ø)
pennylane/transforms/draw.py 100.00% <ø> (ø)
pennylane/__init__.py 100.00% <100.00%> (ø)
pennylane/drawer/__init__.py 100.00% <100.00%> (ø)
pennylane/drawer/mpldrawer.py 98.66% <100.00%> (+0.13%) ⬆️
pennylane/drawer/style.py 100.00% <100.00%> (ø)
pennylane/drawer/tape_mpl.py 100.00% <100.00%> (ø)
pennylane/ops/channel.py 100.00% <100.00%> (ø)
... and 2 more

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 06a15ff...71b2764. Read the comment docs.

@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava, you're right using the Kraus Matrices would blow up very fast. This is my first suggestion how to solve this.

I suggest returning a dictionary containing an entry for every wire up to the maximum wire specified. Each of the operators which are specified in the operators string are applied to the wire specified in the corresponding position of the wires list. This is done by appending the two according Kraus Matrices to a list at the specified position in the dictionary. If a wire didn't receive any operators its according value in the dictionary is None. Note, that this solution also allows the application of multiple gates on one wire (do I need to adapt the probabilities then?).

For now it is only possible to specify one string of operators, one list of wires and one probability value. An extension could maybe return a list of dictionaries...

I hope this points into the direction you were looking for and I'm excited about your opinion on this solution

@antalszava antalszava self-requested a review October 25, 2021 22:18
@antalszava
Copy link
Contributor

Hi @MoritzWillmann, I've been giving this a bit more thought and also started considering the integration with default.mixed. Thank you for your ideas on possible implementation approaches. 👏

The approach that you present sounds like something I had in mind. However, this approach will also likely need significant changes to default.mixed. In specific, default.mixed._apply_channel method expects:

  • All the Kraus matrices at once;
  • All Kraus matrices to target the same wire.

To use the representation that you've proposed and I had in mind, we would need to change _apply_channel and also include specialized cases to make up for applying operators corresponding to c * I on wires that were "not considered" in the Kraus matrices (where c is a normalization constant coming from the constraint of keeping channels trace-preserving maps). So altogether, will likely need more machinery on the simulator side.

So what would likely fit the existing API best after all is to compute the dense Kraus matrices and pass those to the default.mixed._apply_channel method. To solve the original issue and get a working version in, this should be okay. We could add a note box to the channel that the approach is computationally intensive for larger systems because it uses Kraus matrices over all targeted subsystems. If this addition is in, that will already provide a nice convenience to add dedicated errors in smaller simulations and can be iterated over further from a performance perspective in the future.

Let me know what you think!

@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava,

I totally agree that any solution right now is suboptimal. I can implement it in the original way with the dense Kraus matrices and add a notice (and maybe an upper limit for wires?). I'll also check later if it's possible to use any sparse matrix representation. I think this doesn't make a whole lot of sense though because matrix density should be quite high if p is not 0 or 1.

Cheers Moritz

@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava,
I'm having troubles accessing the wires supplied to the Channel Class. I get the error

AttributeError: 'property' object has no attribute 'tolist'

when trying to access them with

cls.wires.tolist()

cls.wires should return a Wiresobject on which I should be able to call .tolist(), or am I missing something?

Cheers Moritz

@antalszava
Copy link
Contributor

Hi @MoritzWillmann, yes, this is something new related to qml.PauliError. Previously channels either had fixed wires or didn't have a dependency of wires when creating the Kraus matrices.

The wires is a property of the instance, whereas cls refers to the class object itself. In order to query the channel.wires, channel needs to be an instance of a channel class, instead of the specific class object itself.

In [2]: qml.BitFlip(0.3, wires=0)
Out[2]: BitFlip(0.3, wires=[0])

In [3]: op = qml.BitFlip(0.3, wires=0)

In [4]: op.wires
Out[4]: <Wires = [0]>

In [5]: op = qml.BitFlip.wires

In [6]: op
Out[6]: <property at 0x7f49218e3540>

Even perhaps with qml.PauliError, we could get away with using other means than querying the wires instance attribute: every parameter that the user passes will be passed to _kraus_matrices via the params argument. For that reason, the third list of arguments passed to qml.PauliError should contain the wires.

Another avenue to go down might be to make _kraus_matrices an instance method of qml.PauliError instead of a clasd method. This avenue might just lead to some subtleties to work out.

On another note, the __init__ method of the QubitChannel channel can be a good pointer on how input validation could work for the PauliError class.

r"""PauliError(operators, wires, p)
Arbitrary number qubit, arbitrary Pauli error channel.

This Class returns a dictionary of Kraus Matrices specified in the operators string.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor thing: it's the _kraus_matrices method that will return the matrices (in some format).

@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava,
Thanks a lot for the quick reply. I figured using the instances wires property would be more elegant than adding another parameter but I see why it won't work. I'll finish the implementation and add some tests so maybe we can wrap this up soon 😃

@MoritzWillmann MoritzWillmann changed the title [WIP] Create qml.PauliError Create qml.PauliError Oct 29, 2021
@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava, this is my final suggestion for now. Let me know what you think 🙂

@MoritzWillmann
Copy link
Contributor Author

hey @antalszava, sorry again for the large delay. I added the suggested changes. I'll start adding the tests for default.mixed

@MoritzWillmann
Copy link
Contributor Author

Hey @antalszava, can you take a look at the tests. I'm not 100% sure about them but I tried to cross check with Quirk as good as I can. Cheers Moritz

@antalszava
Copy link
Contributor

Hi @MoritzWillmann, sure! I'll be taking a look and leave a note here.

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
p (float): The probability of the operator being applied
wires (Sequence[int] or int): The wires the channel acts on

**Example:**
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! :)

pennylane/ops/channel.py Outdated Show resolved Hide resolved
pennylane/ops/channel.py Outdated Show resolved Hide resolved
pennylane/ops/channel.py Outdated Show resolved Hide resolved
def _kraus_matrices(cls, *params):
operators, p = params[0], params[1]

nq = len(operators)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
nq = len(operators)
nq = len(self.wires)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, that's the same problem we had in the beginning

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, good catch! We don't have self here indeed.

tests/devices/test_default_mixed.py Outdated Show resolved Hide resolved
]

@pytest.mark.parametrize("ops", diagonal_ops)
def test_diagonal_kraus(self, ops, tol):
"""Tests that matrices of non-diagonal unitary operations are retrieved correctly"""
"""Tests that matrices of diagonal unitary operations are retrieved correctly"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

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 @MoritzWillmann, this is amazing! 😍 Overall all looks good, my comments are minor suggestions. 👍 🙂 Thank you for coming back and adding the remaining test cases.

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! Thank you so much for this great addition @MoritzWillmann and for hanging in there throughout the process 🎉 😊

@antalszava antalszava merged commit 71de58c into PennyLaneAI:master Dec 6, 2021
@MoritzWillmann
Copy link
Contributor Author

It was a pleasure 💯

@MoritzWillmann MoritzWillmann deleted the pauli-error branch December 6, 2021 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Hacktoberfest not merged but in a good shape
Projects
None yet
3 participants