Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add transform to insert operations into circuits #1795
Add transform to insert operations into circuits #1795
Changes from 51 commits
c470e10
76eb9c8
68494fd
801496c
36212a4
3e5057a
8575a59
c0ad944
887f878
e94095a
e50402c
f738e39
8e2ef87
73dbc9f
89feb7a
9433575
1ade5b3
a4d64f3
2f1bbd8
5940470
807b33a
31e5b2c
7aed33c
59b6f9e
d06d66f
de8594d
71a7075
7406919
f31ac72
b4e512a
27e4666
c110d9a
7777df6
dfb0cc1
f0854e0
21cc485
7989ed0
ca834bf
fad9028
9fadacd
64c2cc2
ce7468a
b24adb8
c9517fa
c9ff037
441d6ed
c48ed3a
ec83d76
55a733b
6c5bfba
ae204f8
1e3b18a
a3b3248
4f71fb9
dda77e3
5dead27
171f6ef
eca476b
540f253
94c69db
42ea5bd
c3cca25
e23ff95
503f2d2
ee23212
788a68b
c76abaa
62c58b5
96cb3d0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for restricting to single-qubit operations? I think a key use-case of this feature would be for tinkering with the noise on, e.g., adding depolarization to all the CNOTs in a circuit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with the single-qubit restriction on the inserted operation just to make placement easier. For example, if the inserted operation is
CNOT
(which wouldn't currently be allowed) andposition="all"
, I'm not sure where we'd place the second wire after, e.g., aHadamard
gate.On the other hand, we can insert single-qubit gates in circuits that already include two-qubit gates. So we could place depolarizing noise after a CNOT, but just not selectively yet (i.e., it'd be after all gates).
I agree though, a bit more flexibility in this function would be great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be useful too to add an option for all gates of a certain type; e.g., if you just wanted to add depolarizing noise to a particular gate, or different amounts of noise to different types of gates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, definitely agree with these points! I did initially think about having the
position
argument be a mapping from gate to inserted gate, but decided to keep things simple for an initial attempt.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps mention here the interaction with state prep operations and
'start'
option. (Or maybe better in usage details?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, made that change: 96cb3d0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not blocking approval, but I was staring at this trying to come up with a better name, when I realized that the signature is almost identical to the transform above --- only differing in the type of the first argument.
This makes it the perfect use-case for
@functools.singledispatch
, which would then allowand
which solves the naming!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, one transform to rule them all! This is cool, am checking out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come
execute
is mentioned here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this line I'm trying to say:
pennylane.beta.QNode
execute()
function (rather than thetape.execute()
method, which won't work becauseexpand_fn
is not called.Think it needs a reword? 🤔 Or maybe I have misunderstood?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it was the 'and',
which confused me, made me think both needed to be used to run this transform.
Even though it is not the full information, I would probably truncate this to
since QNodes will be 99% of use-cases, and
tape.execute()
will be removed shortly anyway.