Feat cirq custom gate - #200
Conversation
| changed = _gray_code(i) ^ _gray_code(i + 1) | ||
| control = next(i for i in range(len(thetas)) if (changed >> i & 1)) | ||
| control = max(-control - position - 1 + circuit.nb_qubits, 1) | ||
| control = max(-control - targets[position] - 1 + circuit.nb_qubits, 1) |
There was a problem hiding this comment.
linked to previous comment
There was a problem hiding this comment.
@MathieuG-Colibri can you give an explanation why we needed to introduce targets[position]?
Co-authored-by: Henri de Boutray <133855265+Henri-ColibrITD@users.noreply.github.com>
| targets.sort() | ||
|
|
||
| return quantum_shannon_decomposition( | ||
| rearrange_matrix(self.matrix, self.targets, do_copy=True), targets |
There was a problem hiding this comment.
I checked the tests of rearrange_matrix, it looks like they are not really independent from the specific implementation we made, since it reuses it behind the usage of CustomGate, I would add some specific "manual" examples to be 100% sure.
There was a problem hiding this comment.
Actually Henri wanted the example to be not circuit related because it's in tools/math.
Nevertheless done 9e33b6c
There was a problem hiding this comment.
I'm not reffering to the example in the documentation, I was more talking about tests, in the tests folder. If you look at them, we reuse CustomGate in the test, which makes the comparison in the tests not independant from the feature.
| if job.circuit.input_g_phase: | ||
| import numpy as np | ||
|
|
||
| state_vector = state_vector * np.exp(-1j * job.circuit.input_g_phase) |
There was a problem hiding this comment.
This is normally directly done in Result no?
There was a problem hiding this comment.
Not in the case of cirq (couldn't have known because we didn't implement the cirq gphase before)
There was a problem hiding this comment.
But don't we want to do it in the Result class, like we do for other providers, instead of here?
It would be cleaner if we group this kind of correction to the same place no?
github-actions
Bot
added
the
documentation
| .. automodule:: mpqp.translation.qasm.qasm_to_cirq | ||
|
|
||
|
|
||
| Direct translations |
There was a problem hiding this comment.
I would be careful when using the term "direct", which is more internal and less standard
There was a problem hiding this comment.
Apparently not done
| G = G0 @ G1.conj().T | ||
| g = G0 @ G1.conj().T | ||
| G = np.round(g, 10) | ||
| eigvals, v = np.linalg.eig(G) |
There was a problem hiding this comment.
So we compute the eigenvalues and vectors on rounded (potentially non-unitary) matrix G, before closest unitary ?
Also here in line 67 you compute eigenvectors of G that you store in v, but then you overrite vto put the closest unitary of itself, which means that nows the eigvals may not be exactly the ones of eigvals, can this cause a problem?
There was a problem hiding this comment.
Up to e-10 it doesn't make much of a difference, it's ugly and arbitrary but it works well still.
There was a problem hiding this comment.
I would put a TODO or a FIXME in the code, because it can potentially numerically cause some problems, especially for larger matrices
Co-authored-by: hJaffaliColibritd <133856040+hJaffaliColibritd@users.noreply.github.com>
Co-authored-by: hJaffaliColibritd <133856040+hJaffaliColibritd@users.noreply.github.com>
Co-authored-by: hJaffaliColibritd <133856040+hJaffaliColibritd@users.noreply.github.com>
4 participants
Change the cirq custom gate implem and behavior
Cleanup and bug fixing
Refactored to_other_language