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

allow creating a prod op with a qfunc #4011

Merged
merged 12 commits into from
May 3, 2023
Merged

allow creating a prod op with a qfunc #4011

merged 12 commits into from
May 3, 2023

Conversation

timmysilv
Copy link
Contributor

@timmysilv timmysilv commented Apr 18, 2023

Context:
Quantum functions (aka qfuncs) are helpful ways of encapsulating a collection of operators. So is the new Prod operator!

Description of the Change:
Users can pass a qfunc to qml.prod in order to effectively convert from a qfunc to a Prod

Benefits:
qfuncs are loosely defined, since they are vanilla python callables and PennyLane can't do much with them. Prods are PennyLane operators and generally much more powerful objects. This will hopefully make stuff easier for users as well.

Possible Drawbacks:
Errors/behaviour can get uglier when mis-used, but I don't think it's worth the extra type-checking (lmk if you want it though). Here are some examples to open discussion:

  1. The wire order might not be what the user expects because we reverse the order of operators to create the Prod:
def qfunc():
    qml.PauliX(0)
    qml.PauliY(1)

op = qml.prod(qfunc)()
print(op, op.wires)
# (PauliY(wires=[1]) @ PauliX(wires=[0]), <Wires = [1, 0]>)
  1. This raises AttributeError: 'function' object has no attribute 'wires' because we can't combine function arguments with operators:
def qfunc():
    qml.PauliX(0)
    qml.PauliY(1)

qml.prod(qfunc, qml.PauliZ(2))
  1. ParametrizedEvolution is an Operator but it's also a callable, so prod would confuse it as a qfunc if passed on its own (which used to fail saying you need multiple operands), and now it just returns another wrapper whose behaviour honestly confuses me:
f1 = lambda p, t: np.sin(p * t)
H = f1 * qml.PauliY(0)
ev = qml.evolve(H)
qml.prod(ev)([[1.1, 2.2], 2], t=2.)
# <function pennylane.pulse.parametrized_evolution.prod.<locals>.wrapper(params, t, return_intermediate=None, complementary=None, **odeint_kwargs)>
  1. The behaviour isn't quite like qml.adjoint because it can accept an operator or a function. qml.adjoint(qfunc()) also fails like prod, but qml.adjoint(some_op()) doesn't fail. There's no analog for this with prod.
>>> print(qml.adjoint(qml.RX(1.1, wires=0)))
Adjoint(RX(1.1, wires=[0]))
>>> print(qml.adjoint(qml.RX)(1.1, wires=0))  # should this syntax.. not be used? is this an unintended side-effect?
Adjoint(RX(1.1, wires=[0]))
>>> print(qml.prod(qfunc)())
PauliY(wires=[1]) @ PauliX(wires=[0])
>>> print(qml.prod(qfunc()))
# ValueError: Require at least two operators to combine; got 1

here's the link to page in docs if you wanna see the example rendered

@github-actions
Copy link
Contributor

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.

@timmysilv timmysilv requested review from Jaybsoni, trbromley and a team April 18, 2023 20:06
@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #4011 (43561e4) into master (c3206fc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4011   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files         345      345           
  Lines       30726    30737   +11     
=======================================
+ Hits        30639    30650   +11     
  Misses         87       87           
Impacted Files Coverage Δ
pennylane/ops/op_math/prod.py 100.00% <100.00%> (ø)

@timmysilv
Copy link
Contributor Author

[sc-37068]

Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Bringing in @trbromley for this discussion. What do we think about the syntax:

qml.prod(qfunc)(*args, **kwargs)

Yes in this case we will return a function instead of a Prod operator, but we do support this behaviour in other places already, like adjoint.

If we don't want to have the same function with different arguments and outputs, maybe we could just create a new function qml.qfunc_to_opeator, or something else better named?

@trbromley
Copy link
Contributor

Bringing in @trbromley for this discussion. What do we think about the syntax:

qml.prod(qfunc)(*args, **kwargs)

Yes in this case we will return a function instead of a Prod operator, but we do support this behaviour in other places already, like adjoint.

If we don't want to have the same function with different arguments and outputs, maybe we could just create a new function qml.qfunc_to_opeator, or something else better named?

+1 to having qml.prod() accept a qfunc and return another callable that returns a Prod when called.

@DSGuala @Jaybsoni will this help solve the issue with using qfuncs in qml.QSVT?

@timmysilv
Copy link
Contributor Author

timmysilv commented Apr 19, 2023

sounds good. I've already updated the PR to match that, but I also updated the 4 cases of concern I thought of. One highlights why this isn't the same as adjoint

@timmysilv
Copy link
Contributor Author

the behaviour surrounding single-op qfuncs (and single-operand Prods in general) is too varied, and this concept (converting from some input type to a single operator) needs more consideration. We will pick this up in a new manner for the next release

@timmysilv timmysilv closed this Apr 21, 2023
@timmysilv timmysilv reopened this Apr 25, 2023
@timmysilv timmysilv requested review from albi3ro and mudit2812 April 26, 2023 14:53
@timmysilv timmysilv added the review-ready 👌 PRs which are ready for review by someone from the core team. label Apr 26, 2023
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

I like this solution and think it will end up being very useful.

Copy link
Contributor

@mudit2812 mudit2812 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 to me

@timmysilv timmysilv enabled auto-merge (squash) May 3, 2023 20:47
@timmysilv timmysilv merged commit 755b241 into master May 3, 2023
@timmysilv timmysilv deleted the prod-accepts-qfunc branch May 3, 2023 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants