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

Make it easier to assign parameters to vector of parameters #5557

Closed
nonhermitian opened this issue Dec 23, 2020 · 6 comments · Fixed by #5759
Closed

Make it easier to assign parameters to vector of parameters #5557

nonhermitian opened this issue Dec 23, 2020 · 6 comments · Fixed by #5759
Labels
type: enhancement It's working, but needs polishing

Comments

@nonhermitian
Copy link
Contributor

nonhermitian commented Dec 23, 2020

What is the expected enhancement?

It is currently a struggle to figure out how to bind a vector of parameters to a circuit. There is no example in the tutorials for doing this (that I could find) and the only example that I could make work is the following:

import numpy as np
from qiskit.circuit.library import EfficientSU2

num_qubits = 4
qc = EfficientSU2(num_qubits, reps=1, parameter_prefix='th')
qc.draw()
     ┌───────────┐┌───────────┐               ┌───────────┐┌────────────┐»
q_0: ┤ RY(th[0]) ├┤ RZ(th[4]) ├──■────■────■──┤ RY(th[8]) ├┤ RZ(th[12]) ├»
     ├───────────┤├───────────┤┌─┴─┐  │    │  └───────────┘└────────────┘»
q_1: ┤ RY(th[1]) ├┤ RZ(th[5]) ├┤ X ├──┼────┼────────■────────────■───────»
     ├───────────┤├───────────┤└───┘┌─┴─┐  │      ┌─┴─┐          │       »
q_2: ┤ RY(th[2]) ├┤ RZ(th[6]) ├─────┤ X ├──┼──────┤ X ├──────────┼───────»
     ├───────────┤├───────────┤     └───┘┌─┴─┐    └───┘        ┌─┴─┐     »
q_3: ┤ RY(th[3]) ├┤ RZ(th[7]) ├──────────┤ X ├─────────────────┤ X ├─────»
     └───────────┘└───────────┘          └───┘                 └───┘     »
«                                              
«q_0: ─────────────────────────────────────────
«     ┌───────────┐┌────────────┐              
«q_1: ┤ RY(th[9]) ├┤ RZ(th[13]) ├──────────────
«     └───────────┘├────────────┤┌────────────┐
«q_2: ──────■──────┤ RY(th[10]) ├┤ RZ(th[14]) ├
«         ┌─┴─┐    ├────────────┤├────────────┤
«q_3: ────┤ X ├────┤ RY(th[11]) ├┤ RZ(th[15]) ├
«         └───┘    └────────────┘└────────────┘
bind_dict = {}
for key in qc.parameters:
    bind_dict[key] = np.random.random()

qc.bind_parameters(bind_dict).draw()
     ┌───────────────────────┐┌───────────────────────┐                »
q_0: ┤ RY(0.328180907065503) ├┤ RZ(0.415892526259694) ├───■────■────■──»
     ├───────────────────────┤├───────────────────────┤ ┌─┴─┐  │    │  »
q_1: ┤ RY(0.366501602030528) ├┤ RZ(0.612414232855608) ├─┤ X ├──┼────┼──»
     ├───────────────────────┤├───────────────────────┤ └───┘┌─┴─┐  │  »
q_2: ┤ RY(0.625750425682392) ├┤ RZ(0.395682286691003) ├──────┤ X ├──┼──»
     ├───────────────────────┤├───────────────────────┴┐     └───┘┌─┴─┐»
q_3: ┤ RY(0.183319402117168) ├┤ RZ(0.0601202730114848) ├──────────┤ X ├»
     └───────────────────────┘└────────────────────────┘          └───┘»
«     ┌───────────────────────┐┌───────────────────────┐»
«q_0: ┤ RY(0.590817320505799) ├┤ RZ(0.583043476311747) ├»
«     └───────────────────────┘└───────────────────────┘»
«q_1: ────────────■────────────────────────■────────────»
«               ┌─┴─┐                      │            »
«q_2: ──────────┤ X ├──────────────────────┼────────────»
«               └───┘                    ┌─┴─┐          »
«q_3: ───────────────────────────────────┤ X ├──────────»
«                                        └───┘          »
«                                                       »
«q_0: ──────────────────────────────────────────────────»
«     ┌───────────────────────┐┌───────────────────────┐»
«q_1: ┤ RY(0.349039573341165) ├┤ RZ(0.735702696793504) ├»
«     └───────────────────────┘├───────────────────────┤»
«q_2: ────────────■────────────┤ RY(0.449403086247425) ├»
«               ┌─┴─┐          ├───────────────────────┤»
«q_3: ──────────┤ X ├──────────┤ RY(0.580120052649959) ├»
«               └───┘          └───────────────────────┘»
«                              
«q_0: ─────────────────────────
«                              
«q_1: ─────────────────────────
«     ┌───────────────────────┐
«q_2: ┤ RZ(0.962327015751139) ├
«     ├───────────────────────┤
«q_3: ┤ RZ(0.426153889936592) ├
«     └───────────────────────┘

However this is confusing because one is temped to do either qc.bind_parameters({'th': np.random.random(15)}) or qc.bind_parameters({ParameterVector('th', 15): np.random.random(15)}), both of which do not work. Moreover, inputting any key value one wants does not raise an exception, which makes things even more difficult to debug.

Finally the returned parameters in qc.parameters are not in numeric order that also makes things a mess, e.g."

{Parameter(th[0]),
 Parameter(th[10]),
 Parameter(th[11]),
 Parameter(th[12]),
 Parameter(th[13]),
 Parameter(th[14]),
 Parameter(th[15]),
 Parameter(th[1]),
 Parameter(th[2]),
 Parameter(th[3]),
 Parameter(th[4]),
 Parameter(th[5]),
 Parameter(th[6]),
 Parameter(th[7]),
 Parameter(th[8]),
 Parameter(th[9])}
@nonhermitian nonhermitian added the type: enhancement It's working, but needs polishing label Dec 23, 2020
@Cryoris
Copy link
Contributor

Cryoris commented Dec 23, 2020

If you do

qc.bind_parameters(qc.ordered_parameters: np.random.random(15)}

I think that should work. But yeah I agree it's cumbersome (especially bc that ordered_parameters only exists on the NLocal circuits bc they were needed for consistency & backward compatibility on the variational algos). However we're looking to make parameters sorted and then we could facilitate binding

@nonhermitian
Copy link
Contributor Author

Yeah, so it does seem that some circuits in the circuit library accept arrays as inputs to assign-parameters, but it is not universal. Namely the main circuit class does not. This leads to confusion and breaking code because one could pass an array in one case and then try their own circuit and it would fail. So currently the situation is that either dicts are used everywhere, which as stated above is not the easiest thing to do currently, or one has to know the implementation details of Qiskit for knowing when a "circuit" is one that takes a array or not; There is no unique def for quantum circuit methods in Qiskit at the moment.

@paolob67
Copy link
Contributor

Hi. I created this quick patch for addressing this enhancement request.

diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py
index 5ef05744..0e435d3d 100644
--- a/qiskit/circuit/quantumcircuit.py
+++ b/qiskit/circuit/quantumcircuit.py
@@ -2035,7 +2035,29 @@ class QuantumCircuit:
         """
         if any(isinstance(value, ParameterExpression) for value in value_dict.values()):
             raise TypeError('Found ParameterExpression in values; use assign_parameters() instead.')
-        return self.assign_parameters(value_dict)
+        parsed_value_dict = {}
+        # allow for key,value pair types other than Parameter(),value in the value_dict
+        # so that parameter vectors can be assigned easily
+        for the_param_key in value_dict.keys():
+            param_name = ''
+            # handle 'str,array()' pair type where str is the name of the vector
+            if isinstance(the_param_key, str):
+                param_name = the_param_key
+            # handle 'ParameterVector(),array()' pair type
+            elif isinstance(the_param_key, ParameterVector):
+                param_name = the_param_key.name
+            # if param_name is set then parse value dict
+            if param_name:
+                the_values = value_dict[the_param_key]
+                for i in range(len(the_values)):
+                    print(i)
+                    for key in self.parameters:
+                        if key.name == '{}[{}]'.format(param_name, i):
+                            parsed_value_dict[key] = the_values[i]
+            # otherwise just copy the item
+            else:
+                parsed_value_dict[the_param_key] = value_dict[the_param_key]
+        return self.assign_parameters(parsed_value_dict)
 
     def _unroll_param_dict(self, value_dict):
         unrolled_value_dict = {}

It seems to work on the proposed scenario. Let me know if this would be a viable code change and I'll be happy to work on a PR.

@Cryoris
Copy link
Contributor

Cryoris commented Jan 10, 2021

I agree that we should enhance assigning parameters to circuits, right now it can be quite cumbersome. But I think we should properly discuss how we want to do that, such that we don't end up with too many different possibilities.

The simplest manner to assign an array of values, in my opinion would just be

# ``circuit`` contains some parameters
values = [0, 1, 2]
bound_circuit = circuit.assign_parameters(values) 

This would be very convenient for usage with optimizers and optimization algorithms in Qiskit, such as the VQE.

If you want to only bind certain parameters, we could use the current scheme

partially_bound = circuit.assign_parameters({x: value})

Personally I'd be fine if that'd work with a string as well, since parameter names are unique in circuits.

partially_bound = circuit.assign_parameters({'x': value})

The first 'anonymous' assign to just an array requires a notion of parameter order. We don't have that at the moment but we're working on this with @ewinston 🙂

So I think the following would cover @nonhermitian cases and would be very user-friendly to go forward:

  • once parameters are ordered, allow assigning a plain array: assign_parameters(values)
  • allow assigning per instance or name: assign_parameters({param: value, ...})

We could split this in two PRs and add assign per name now and the array-assign later. But we should get more opinions on if this could break some things, e.g. from @kdk

@kdk
Copy link
Member

kdk commented Jan 14, 2021

Hi. I created this quick patch for addressing this enhancement request.

diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py
index 5ef05744..0e435d3d 100644
--- a/qiskit/circuit/quantumcircuit.py
+++ b/qiskit/circuit/quantumcircuit.py
@@ -2035,7 +2035,29 @@ class QuantumCircuit:
         ...         

It seems to work on the proposed scenario. Let me know if this would be a viable code change and I'll be happy to work on a PR.

Thanks @paolob67 , this looks like a good approach. Can you open a PR and we can discuss it further there?

@paolob67
Copy link
Contributor

Yes no prob. I'll work on adding a couple of test cases and then I'll create a PR. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
4 participants