New UI for passing Parameter values

[lillian542]

Old UI continues to be supported:

# Qiskit circuit
from qiskit import QuantumCircuit
from qiskit.circuit import Parameter

theta = Parameter("theta")
phi = Parameter("phi")
psi = Parameter("psi")

qc = QuantumCircuit(1,1)
qc.rx(theta*phi, [0])
qc.rz(psi, [0])

# Conversion to PennyLane
dev = qml.device("default.qubit")

@qml.qnode(dev)
def circuit(x, y, z):
    qml.from_qiskit(qc)(params={theta: x, phi: y, psi:z})
    return qml.expval(qml.PauliZ(0))

or

qml.from_qiskit(qc)({theta: x, phi: y, psi:z})

but the new reccommended UI is also implemented, with options to pass as args, kwargs, or a combination:

qml.from_qiskit(qc)(y, z, x)  # order is alphabetical wrt parameter names
qml.from_qiskit(qc)(theta=x, phi=y, psi=z)
qml.from_qiskit(qc)(z, theta=x, phi=y)

If params are passed that don't match, we get clear errors for too many arguments:

>>> qml.from_qiskit(qc)(1, 2, 3, 4)
TypeError: Expected 3 positional arguments but 4 were given

>>> qml.from_qiskit(qc)(1, 2, 3, theta=4)
TypeError: Expected 2 positional arguments but 3 were given

for not enough arguments:

>>> qml.from_qiskit(qc)()
TypeError: Missing required arguments to define Parameter values for: phi, psi, theta

>>> qml.from_qiskit(qc)(phi=0.2, psi=0.3)
TypeError: Missing 1 required argument to define Parameter value for: theta

>>> qml.from_qiskit(qc)(0.2)
TypeError: Missing 2 required arguments to define Parameter values for: theta, psi

and unrecognized key word arguments:

>>> qml.from_qiskit(qc)(phi=0.2, psi=0.3, misspelled_kwarg=0.4)
TypeError: Got unexpected parameter keyword argument 'misspelled_kwarg'. Circuit contains parameters: phi, psi, theta

[lillian542]

[sc-56075]

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ff2e352) 100.00% compared to head (eaf8256) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #406   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          331       362   +31     
=========================================
+ Hits           331       362   +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[trbromley]

qml.from_qiskit(z, theta=x, phi=y)

To confirm, is this part a typo that should be qml.from_qiskit(qc)(z, theta=x, phi=y)?

[lillian542]

To confirm, is this part a typo that should be qml.from_qiskit(qc)(z, theta=x, phi=y)?

Yes!

[obliviateandsurrender]

Thanks @lillian542! Looks great, just leaving some initial feedback.

[trbromley]

Thanks @lillian542 @obliviateandsurrender!

qml.from_qiskit(qc)(y, z, x)  # order is alphabetical wrt parameter names

Could we get the order to depend on the order specified in the Qiskit circuit? I worry that alphabetical could be less intuitive.

>>> qml.from_qiskit(qc)(phi=0.2, psi=0.3)
TypeError: Missing 1 required argument to define Parameter values for: {Parameter(theta)}

Would it make sense if it were instead TypeError: Missing 1 required argument to define Parameter values for: theta? Is there a motivation to wrap it in a set?

>>> qml.from_qiskit(qc)(phi=0.2, psi=0.3, misspelled_kwarg=0.4)
TypeError: Got unexpected parameter keyword argument 'misspelled_kwarg'. Circuit contains parameters ParameterView([Parameter(phi), Parameter(psi), Parameter(theta)])

Not a huge blocker, but I'm not so keen on the repr for ParameterView([Parameter(phi), Parameter(psi), Parameter(theta)]).

[lillian542]

Could we get the order to depend on the order specified in the Qiskit circuit? I worry that alphabetical could be less intuitive.

We could store them in the order they are used when defining and executing the Qiskit circuit. Here's why I didn't: The qiskit circuit stores parameters as qc.parameters alphabetically. If the user queries the Qiskit circuit for parameters, they will be sorted like this. If we take them in a different order:

1. our order won't match qc.parameters
2. we will have to generate the list of parameters in a less reliable, more hacky way, like using protected methods (qc._parameter_table), or iterating over the list of instructions, pulling the parameters out, and looping over to break down ParameterExpression elements until there are only Parameter and ParameterVectorElements left.

Let me know what you think. The order they are used is an option.

Would it make sense if it were instead TypeError: Missing 1 required argument to define Parameter values for: theta? Is there a motivation to wrap it in a set?

There's no motivation, it's just already a set when it gets to this point in the code, and I used it as is. I'll change them all to use just the names of the parameters. This also covers the final comment about the ParameterView repr - its just used as-is right now, I'll clean it up!

[trbromley]

Ah I did mean to use the order specified by qc.parameters, I just didn't realize that that would be alphabetical! So how it's currently done sounds great.

[obliviateandsurrender]

Just some docs suggestions, the rest looks good to me!

[obliviateandsurrender]

Why does codefactor complain about this function? 🤔

[albi3ro]

So pylint and sourcery are definitely complaining about this file, but they don't seem to be concerned about this function. And it doesn't seem particularly nested or branchy to me... 🤔

[albi3ro]

I wonder if its placing the warning in the wrong place, since _function is overly nested, branchy, and overall low code quality.

[lillian542]

I'm going to ignore it for now, looks like the CodeFactor check still passes, and there are quite a few changes to this function in the next PR - I don't really want to create a bunch of merge conflicts by moving things around here.

[albi3ro]

I wonder if its worth moving these warnings and examples into the docstring for load, as that is the docstring that will render in the website.

[lillian542]

I think we definitely want them here, because this is what you get if you "ask" the function what it does in Jupyter, i.e.

qfunc = qml.from_qiskit(qc)
qfunc?

I think we want something more extensive in qml.from_qiskit, which will be the documentation on the PennyLane website. @trbromley , do we also want to expand documentation on the load function in the plugin, or is that more internal?

[trbromley]

I like that you can query qfunc in Jupyter and get that info! And yes we do want the from_qiskit docs to be on point - though we can treat it as a follow up to revisit those docs. I think the load function in the plugin can be made a copy of whatever is in from_qiskit.

[albi3ro]

Potentially may need to silence code factor, but my comments are purely optional and non-blocking. Looks like a much cleaner syntax :)