Custom boolean functions for supporting noise models #5674
Conversation
|
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5674 +/- ##
==========================================
- Coverage 99.68% 99.67% -0.01%
==========================================
Files 415 417 +2
Lines 38898 38748 -150
==========================================
- Hits 38774 38623 -151
- Misses 124 125 +1 ☔ View full report in Codecov by Sentry. |
|
Thanks @obliviateandsurrender! Would you mind linking the corresponding SC story? |
| .. seealso:: Users are advised to use :func:`~.wires_eq` for a functional construction. | ||
| """ | ||
|
|
||
| def __init__(self, wires): |
There was a problem hiding this comment.
I might be misremembering, but I thought I saw you show in the meeting the input argument of WiresEq being an operation?
There was a problem hiding this comment.
Oh it's in the wires_in and wires_eq function below 🤔 Why do we need operation input?
There was a problem hiding this comment.
This is just an additional support to make the constructor functions more flexible and easy to use for the users. Especially in a scenario where they are trying to automate building noise models based on some set of operations they have in their pipeline, then they can still make these conditional without having to extract wires manually.
pennylane/noise/conditionals.py
Outdated
| ops (str, Operation, Union(list[str, Operation])): string | ||
| representation or instance of the operation. |
There was a problem hiding this comment.
Here I was expecting op_in([qml.RX, qml.Hadamard]). Is this approach challenging/not advisable?
There was a problem hiding this comment.
I think I just forgot to mention it, but doing that is possible as well! I'll add it to the docs. 😁
| >>> func = qml.noise.partial_wires(qml.RX(1.2, [12])) | ||
| >>> func(2) | ||
| qml.RX(1.2, wires=[2]) |
There was a problem hiding this comment.
I'm confused, so we originally have an instantiated RX on wire 12 but it gets mapped to wire 2?
There was a problem hiding this comment.
Yes. We had an operation that was RX(1.2, wires=12). When we wrap it with a partial_wires, we can instantiate that operation with the new input wires, in this case, 2.
| >>> func = qml.noise.partial_wires(qml.RX(1.2, [12])) | ||
| >>> func(2) | ||
| qml.RX(1.2, wires=[2]) | ||
| >>> func(qml.RY(1.0, ["wires"])) |
There was a problem hiding this comment.
🤔 Do we need to support calling on operations, rather than just wires?
There was a problem hiding this comment.
It makes it easier to use this method and is kinda intuitive as well since we want to initialize the wrapped operation based on the wires of an encountered operation.
There was a problem hiding this comment.
Looking good so far. I added a few suggestions to clean up the code, along with some clarification questions. The two big questions I have:
- The only use of the bitwise and condition properties so far is to determine what the
reprof the BooleanFunction is going to be. If they are just being used as flags, then I can suggest two alternatives that might be cleaner and more space efficient:
-
Update the base
reprmethod to always returnBooleanFn(self.fn.name), and in each of the sub classes, just overwrite thereprmethod directly (as you have done for thestrmethod. You can also drop thenamekwarg. -
Update the base
reprmethod to returnBooleanFn(self.fn.name)ifname is Noneelse returnname. The only time name is provided is when the sub classes are conditionals or bitwise operators. In the case that a user provides a name, we probably want to display that anyways.
- Regarding
OpEq, it doesn't seem to me that we need it? It's just a special case ofOpInwhen the list of operations is length 1. Also it seems to be coded to allow passing in multiple operators both as the reference list and as the items being compared, but this doesn't make sense to me because when we use this conditional in a circuit, we are going to iterate over the gates in the circuit one at a time anyways? So the item being compared will always be a single operation. Furthermore, comparing multiple lists on both sides can be easily achieved by using the map function, so it's probably not worth it? Happy to discuss offline if I am missing something here.
Look forward to seeing this get in!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5674 +/- ##
==========================================
- Coverage 99.67% 99.67% -0.01%
==========================================
Files 416 418 +2
Lines 38686 38543 -143
==========================================
- Hits 38562 38418 -144
- Misses 124 125 +1 ☔ View full report in Codecov by Sentry. |
Not really. I do use them for that, but that is not the intended purpose for them. These properties check the existence of underlying respective attributes that store expressions that BooleanFn uses for evaluation. For the
Yes, it is a special case of |
pennylane/noise/conditionals.py
Outdated
| iters = val if isinstance(val, (list, tuple, set, Wires)) else getattr(val, "wires", [val]) | ||
| try: | ||
| wires = set().union(*((getattr(w, "wires", None) or Wires(w)).tolist() for w in iters)) | ||
| except TypeError: |
There was a problem hiding this comment.
Why do you expect a TypeError here?
There was a problem hiding this comment.
TypeError happens when inputs are classes. Added an additional WireError for unhashable input w to Wires, as well.
| self._cond = set(wires) | ||
| self.condition = self._cond |
There was a problem hiding this comment.
Why do you do this instead of self.condition = set(wires)?
There was a problem hiding this comment.
No specific reason, just consistency with the Op classes. I essentially use cond as a private attribute for the conditionals, which I use in the computation later, whereas the condition attribute is a user-facing attribute that I don't access later on.
| One may give an instance of :class:`Operation <pennylane.operation.Operation>` | ||
| for the ``operation`` argument: | ||
| >>> func = qml.noise.partial_wires(qml.RX(1.2, [12])) |
There was a problem hiding this comment.
Probably not super important, but it does not work with some templates:
func = qml.noise.partial_wires(qml.Qubitization(qml.X(0), control = [1]))
It would be the same with any template that has not "wires" directly
There was a problem hiding this comment.
Yeah, it will work with operations (and templates) that take in "wires" directly. It will be too much effort to generalize it IMO as each corner case would be different. Moreover, if a template doesn't take wires directly, there shouldn't be a need to use this.
Context: We aim to have a custom boolean function that could serve as a building block for creating noise models.
Description of the Change:
BooleanFnto work with arbitraryargsandkwargs.NoiseConditionalsas a class that helps buildBooleanFnrequired for creating noise models.wire/op_eq/into enhance the building experience for the aboveBooleanFn.Benefits:
Possible Drawbacks:
Related GitHub Issues: [sc-63080]