Import basic elements from Qibo

[alecandido]

Just to start doing something.

@MatteoRobbiati @stavros11 @hay-k

• fix qibo imports
• add third party dependencies
• (?) fix construct_backend
  • this would require proper revision and discussion, I'm not sure I would put much effort in this initial PR

[alecandido]

Current import-related criticalities:

❯ rg import | rg 'qibo\.' | rg -v 'qibo.backends' | rg -v 'qibo.gates' | rg -v 'qibo.config' | rg -v 'qibo.measurements' | rg -v 'qibo.result'
src/qibo_core/gates/channels.py:        from qibo.quantum_info.superoperator_transformations import (  # pylint: disable=C0415
src/qibo_core/gates/channels.py:        from qibo.quantum_info.superoperator_transformations import (  # pylint: disable=C0415
src/qibo_core/gates/channels.py:        from qibo.quantum_info.basis import comp_basis_to_pauli  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/gates/gates.py:        from qibo.transpiler.decompositions import (  # pylint: disable=C0415
src/qibo_core/__init__.py:from qibo.models.circuit import Circuit

[alecandido]

Transpiler

@stavros11 I would remove the decompose method of the gate, and leave the responsibility to the transpiler alone, for better isolation.

It is not essential to gates to know how they are decomposed (even because it's not unique), and they are also mostly falling back on standard_decomposition (i.e. the import problem we currently have).
This would simplify the dependency.

I would open a corresponding issue in qibo to lift the non-trivial ones (i.e. X, CNOT, and FSWAP gates).

Channels

@renatomello the transformations of a Channel into other representations (Choi, Liouville, Pauli-Liouville) are implemented partial in the class itself and partially in the quantum information module.

I'd be happy to keep the basic definition of the Channel in qibo-core, but could we move the transformations all to the quantum information module? Or would you prefer to have the channel as well in quantum_info?

Circuit

It is actually not referenced anywhere, but I'd be inclined in including it in qibo-core. Do you agree? @stavros11 @hay-k

[renatomello]

Channels

@renatomello the transformations of a Channel into other representations (Choi, Liouville, Pauli-Liouville) are implemented partial in the class itself and partially in the quantum information module.

I'd be happy to keep the basic definition of the Channel in qibo-core, but could we move the transformations all to the quantum information module? Or would you prefer to have the channel as well in quantum_info?

If all representations are removed, than the Channelclass will be missing a method like Gate.matrix, which is an inconsistency.
I'd propose then that either Channel.to_choi or Channel.to_liouville becomes Channel.matrix and to explicitly mention in the documentation that the matrix could be converted to other representations using the functions in the quantum_info module.

[alecandido]

Thanks @renatomello for the answer.

If I understand correctly, Channel.matrix() is already not present, so you currently rely on the explicit choice of one of them from the user (i.e. in current Qibo you either have to call Channel.to_choi() or Channel.to_liouville()).
My proposal is instead to change them from method to functions, such that you will do to_choi(channel) or to_liouville(channel) (to be fair, I'd even drop the to_, choi(channel) seems enough).

The motivation is that the Gate.matrix is quite elementary, since it corresponds to internal representation. For channels this is not the case (in general, they are given in Kraus representation, and stored in self.gates), and all the transformations currently require functions implemented in quantum_info (indeed the imports are inside the functions, to avoid being circular).

The idea is similar to what numpy is doing with the np.linalg module: basic operations are available as methods, more advanced ones are separate functions, used through an explicit access to a further module.

[renatomello]

If I understand correctly, Channel.matrix() is already not present, so you currently rely on the explicit choice of one of them from the user

As of right now, 'Channel.matrix' raises NotImplementedError and redirects the user to one of the other three methods implemented that are representation-specific. But that's just a design choice that can be easily changed.

(i.e. in current Qibo you either have to call Channel.to_choi() or Channel.to_liouville()).

Also to_pauli_liouville.

My proposal is instead to change them from method to functions, such that you will do to_choi(channel) or to_liouville(channel) (to be fair, I'd even drop the to_, choi(channel) seems enough).

Those already exist in quantum_info.superoperator_transformations.

The motivation is that the Gate.matrix is quite elementary, since it corresponds to internal representation. For channels this is not the case (in general, they are given in Kraus representation, and stored in self.gates), and all the transformations currently require functions implemented in quantum_info (indeed the imports are inside the functions, to avoid being circular).

It doesn't really matter to me which representation is returned by Channel.matrix (even though I'd pick Choi for that, Kraus seems to be the easier one for the goal in hand) as long as there is one, just like Gate.matrixreturning a matrix in the computational basis (which is also a non-unique choice of representation in and of itself).

[alecandido]

Also to_pauli_liouville.

Yes, I'm aware of that (mentioned in my first comment above).

Those already exist in quantum_info.superoperator_transformations.

You're right, I just forgot (I now remember some PR about them). So much the better :)

It doesn't really matter to me which representation is returned by Channel.matrix (even though I'd pick Choi for that, Kraus seems to be the easier one for the goal in hand) as long as there is one, just like Gate.matrix returning a matrix in the computational basis (which is also a non-unique choice of representation in and of itself).

About keeping Kraus I agree: I know that is definitely non-unique, as the Gate.matrix is non-unique, but this is true from an abstract perspective. In practice, the moment you're giving numbers you're already in a basis (unless you are just quoting values of invariant observables), so the symmetry is already broken at that level.

Using Kraus would be quite symmetric to the fact that there is no superoperator_transformations.to_kraus (even though it could be even defined, and just ask for Channel.matrix).
Would you take care of writing the Channel.matrix method? (here or in qibo it doesn't matter, if you do it in qibo, I will copy here - and eventually I'm going to import the Channel in qibo from here, but not immediately)

Instead, would you agree on dropping Channel.to_choi(), Channel.to_liouville(), and Channel.to_pauli_liouville? Not in qibo, but just here of course (effective in qibo only after the replacement).

[stavros11]

Thanks @alecandido for starting these efforts. Hopefully this will fix the issues we are having with the circular dependencies (for example qiboteam/qibojit#170 (comment)).

Also now git-blaming this code will point to you, so I will have less things to worry about 😛

Transpiler

@stavros11 I would remove the decompose method of the gate, and leave the responsibility to the transpiler alone, for better isolation.

I fully agree with removing Gate.decompose. In fact, I already raised this point twice in a qibo related PR: here qiboteam/qibo#1188 (review) and here qiboteam/qibo#1188 (review) but from what I understood @renatomello disagrees and in the end it was not removed.

Circuit

It is actually not referenced anywhere, but I'd be inclined in including it in qibo-core. Do you agree? @stavros11 @hay-k

I am not sure about this. I would be inclined to say no, in order to keep qibo-core as minimal as possible. However, even if it is not directly used anywhere, Circuit instances are used for example in the Backend.execute_circuit method, so if we were to enforce typing in all functions at some point, we would need to include it. Therefore, I believe it is better in terms of completeness to have Circuit here.
Another option would be to remove execute_circuit from the Backend, however QibolabBackend needs to overwrite this so I don't think that would work.

Other than these, I guess we should also move some of the qibo tests here and at some point (maybe short before going public) enable CI. I am not sure if you are planning to do this in this PR. (EDIT: Just saw issue #2 about tests)

[renatomello]

Using Kraus would be quite symmetric to the fact that there is no superoperator_transformations.to_kraus (even though it could be even defined, and just ask for Channel.matrix). Would you take care of writing the Channel.matrix method? (here or in qibo it doesn't matter, if you do it in qibo, I will copy here - and eventually I'm going to import the Channel in qibo from here, but not immediately)

I can make the changes in qibo, yes.

Instead, would you agree on dropping Channel.to_choi(), Channel.to_liouville(), and Channel.to_pauli_liouville? Not in qibo, but just here of course (effective in qibo only after the replacement).

Sure, if Channel.matrix replaces it by returning the Kraus representation.

[alecandido]

Also now git-blaming this code will point to you, so I will have less things to worry about 😛

I will manually blame you (or check whether it is possible to tell Git the truth!)

[alecandido]

I am not sure about this. I would be inclined to say no, in order to keep qibo-core as minimal as possible. However, even if it is not directly used anywhere, Circuit instances are used for example in the Backend.execute_circuit method, so if we were to enforce typing in all functions at some point, we would need to include it. Therefore, I believe it is better in terms of completeness to have Circuit here.
Another option would be to remove execute_circuit from the Backend, however QibolabBackend needs to overwrite this so I don't think that would work.

Ok, I see your point. Let's say that I will import Circuit as well for the time being, keeping the "backend package" consistent.
We will also have to revisit the backends implementation (possibly even gates, and so on), so I'd just keep as close as the current qibo for the time being, and postpone this discussion to a later stage (if we rearrange more things, we'll have more freedom in general).

If it's only for the type (in Backend.execut_circuit(), but also in qibocal), what we could do is to have a very minimal Circuit class in qibo-core anyhow, but to move a more extensive circuit library to Qibo (thus out of the class, but if really needed we could hook things dynamically).

Other than these, I guess we should also move some of the qibo tests here and at some point (maybe short before going public) enable CI. I am not sure if you are planning to do this in this PR. (EDIT: Just saw issue #2 about tests)

Yes, this is strictly required asap. However, this PR is not going to be complete in any respect.
I could actually put everything in main directly, but I wanted a board to discuss some decisions with everyone.

Tests & workflows are the next up, since we need the same reliability of qibo to faithfully replace that part (and in a as much as possible self-contained fashion).
Docs will be after that, but I consider them less critical for qibo-core, because it will never be user-facing.

[alecandido]

Ok, the discussion is relevant, but it won't disappear.

But, by now, I'm quite involved in the alternative, so I have no interest in continuing this import.
(if the alternative will fail, we'll be always in time to resume)