Closed khnikhil closed 1 year ago
Hey @khnikhil, this is awesome!
I will still need to go through some of the details, but from an overall glance and some tests, this is what I was looking for. While I go through the details of the PR, something that you can add so long is the following:
So providing a hierarchical object as a mapping doesn't work anymore (check the Sub motif as a mapping for a motif section in the quickstarti.ipynb). Don't worry though, this is an involved issue because I have this Qhierarchy.get_unitary_function in core.py that converts a Qhierarchy object into a Qunitary.function (this is to build up motifs in terms smaller motifs). I have the solution here, just change the following:
On line 1158 in core.py update the get_unitary_function
as follows:
def get_unitary_function(self, **kwargs):
"""
Convert the Qhierarchy into a function that can be called.
"""
def unitary_function(bits, symbols=None, **kwargs):
self.update_Q(bits)
if not (symbols is None):
self.set_symbols(symbols)
return_object = None
for layer in self:
for unitary in layer.edge_mapping:
if isinstance(unitary.function, str):
get_circuit_from_string = kwargs.get("get_circuit_from_string", None)
unitary = get_circuit_from_string(unitary)
return_object = unitary.function(
unitary.edge, unitary.symbols, **kwargs
)
return return_object
return unitary_function
Then in your new qiskit_helper.get_circuit_qiskit function allow it to recieve **kwargs: on line 97
# breaking down the qunitary.function string into a list of gate instructions
instruction_list, unique_bits, _ = get_circ_info_from_string(qunitary.function)
# building a function from the list of gate instructions
def circuit_fn(bits, symbols=None, circuit=None, **kwargs):
qubits = [QuantumRegister(1, bits[i]) for i in range(qunitary.arity)]
Then on line 73 in qiskit_helper.get_circuit_qiskit send your function to the unitary.function call:
circuit = unitary.function(
bits=unitary.edge, symbols=unitary.symbols, circuit=circuit, get_circuit_from_string=get_circuit_from_string
)
The idea here is to send along the backends specific string to circuit conversion function to the abstract get_unitary_function in core.py. Since we want to allow something as follows:
m1 = Qinit(3) + Qpermute(mapping=Qunitary("crx(x_0)^01"))
m2 = Qinit(6) + Qcycle(mapping=m1)
m3 = Qinit(18) + Qcycle(mapping=m2)
We need to make sure whatever that string conversion function is, needs to be accessible on the lowest level of the compute graph. So the only requirement is that if you provide a function, just specify **kwargs which allows us to send around backend specific functions like string to circuit conversions.
One final small change in regards to this is just the default circuits in qiskit_circuits.py, just give each of them a **kwargs parameter:
def U2(bits, symbols=None, circuit=None, **kwargs):
def U3(bits, symbols=None, circuit=None, **kwargs):
def V2(bits, symbols=None, circuit=None, **kwargs):
def V4(bits, symbols=None, circuit=None, **kwargs):
As far as I can tell, with these changes nothing breaks and works as expected.
Tomorrow I will go through the details of the PR, but any changes from here will be small stuff, great job!
Hi @matt-lourens,
Thanks so much for the quick comments! I made the changes you pointed out, and everything seems to be working fine.
Just to make sure I understand, the idea is that we want to pass on the get_circuit_from_string
method through kwargs so that it can be used higher up the stack when we're converting a Qhierarchy
object into a single function?
I'm a bit confused by what you mean by the following:
So providing a hierarchical object as a mapping doesn't work anymore (check the Sub motif as a mapping for a motif section in the quickstart.ipynb).
since in quickstart.ipynb
it seems like the hierarchical object m1
is still being passed as a mapping to hierq
:
Thanks so much for the comments, and please let me know if there are any other fixes that should be made!
Hi @matt-lourens,
Thanks so much for the quick comments! I made the changes you pointed out, and everything seems to be working fine.
Just to make sure I understand, the idea is that we want to pass on the
get_circuit_from_string
method through kwargs so that it can be used higher up the stack when we're converting aQhierarchy
object into a single function?I'm a bit confused by what you mean by the following:
So providing a hierarchical object as a mapping doesn't work anymore (check the Sub motif as a mapping for a motif section in the quickstart.ipynb).
since in
quickstart.ipynb
it seems like the hierarchical objectm1
is still being passed as a mapping tohierq
:![]()
Thanks so much for the comments, and please let me know if there are any other fixes that should be made!
So in regards to this, on the high level (interfacing) you pass the Qhierarchy object as a mapping, but on the low level when a Qmotif is intialised and it's mapping is a Qhierarchy, then the mapping gets updated to a function by calling get_unitary_function()
:
# core.Qmotif
if self.is_default_mapping:
self.arity = 2
else:
if isinstance(self.mapping, Qhierarchy):
# If mapping was specified as sub-hierarchy, convert it to a qunitary
new_mapping = Qunitary(
function=None,
n_symbols=self.mapping.n_symbols,
arity=len(self.mapping.tail.Q),
)
new_mapping.function = self.mapping.get_unitary_function()
self.mapping = new_mapping
self.arity = self.mapping.arity
Before my suggested change when this self.mapping was a Qhierarchy which had a Qmotif
with a mapping as string, the .get_unitary_function() bombs out because that Qmotif
never got it's qiskit converted function to execute. We therefore have to pass that conversion function all the way down (no matter how many layers)during execution, so that the function correctly gets converted to the function we want. It's a bit hard to explain, but it's an interesting problem and solution, if you're interested undo the changes I suggested and debug through the sub_hierq and sub sub hierq examples of the notebook. Then it's a bit more clear what goes wrong :D
@khnikhil This is great! I've left a few comments, but it's mostly about the way we organize the code. In terms of functionality, everything is perfect!
hi @matt-lourens! thanks for the comments - I implemented everything in the new commits. in addition to moving the get_circ_info_from_string
function inside Qunitary
, I changed the name of the function I added to qiskit_helper.py
to get_qiskit_circuit_from_instructions
, which should make the code easier to understand in addition to the extra comments. thanks so much for the comments and support again, this was really fun :)
hi @matt-lourens! I just realized I was mislabelling unique_bits
and unique_params
when calling them in Qunitary.__init__()
which was leading to some bugs, so I went back in and fixed it. Is there anything else I need to do before it can be merged?
hi @matt-lourens! I just realized I was mislabelling
unique_bits
andunique_params
when calling them inQunitary.__init__()
which was leading to some bugs, so I went back in and fixed it. Is there anything else I need to do before it can be merged?
Hey @khnikhil, awesome thanks! Sorry for not getting to this PR yet, I had a crazy end of week last week+weekend.
Hopefully by the end of today I can get around to it, then we can merge :D
this code closes #28 as part of the UnitaryHack 2023 event.
Specifically, I added functionality to build simple circuits from strings (for Qiskit) as shown in the following code example:
circuit_str = "h()^0;CRX(x_0)^01;CRZ(x_1)^10"
hierq = Qinit(5)+Qcycle(mapping=Qunitary(circuit_str))
I show a screenshot comparing this new method with the old method of creating an ansatz function:
The string must be formatted in the following format:
"{gate_string}(parameters)^{bits};{gate_string}(parameters)^{bits};...;{gate_string}(parameters)^{bits}"
wheregate_string
is a Qiskit gate (e.g. one of {h,x,y,z,cx,cy,cz,rx,ry,rz,crx,cry,crz,s,toffoli}) and can be written in uppercase, lowercase, or a mix of cases;parameters
is a string of format {x0, x1, x2, ..., xn}; bits is the qubit/set of qubits on which the gate is applied.NB: Each gate instruction MUST be separated by a semicolon NB: you do not have to specify
arity
andn_symbols
, as these are extracted automatically from the string in the Qunitary__init__()
function.As specified in issue #28, I added code in
core.py
in theQunitary.__init__()
function to check if the input function is a string, and if so, automatically calculate and set thearity
andn_symbols
attributes. Then, I modified theget_circuit_qiskit()
function to convert the string to a Python function, using a helper function inutils.py
.@matt-lourens As this is my first time contributing to an open-source project, please let me know if I'm missing anything! thanks so much :)