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

How to deal with removal of BasicAer? #5718

Closed
t-imamichi opened this issue May 20, 2020 · 30 comments
Closed

How to deal with removal of BasicAer? #5718

t-imamichi opened this issue May 20, 2020 · 30 comments
Labels
mod: algorithms Related to the Algorithms module

Comments

@t-imamichi
Copy link
Member

What is the expected enhancement?

The removal of BasicAer is under discussion.
#4443

There are some Aqua modules that use BasicAer, e.g., CircuitOp.
It would be good to discuss how to deal with the removal.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 21, 2020

I checked c1375ff and found following lines that use BasicAer.
There are two types

  1. BasicAer as backend. run_circuits.py and backend_utils.py
    • Can we replace it with Aer?
    • If so, we need to change requirements.txt to include qiskit-aer
  2. BasicAer is used as a utility to get a state vector, unitary matrix, or counts.
aqua/utils/run_circuits.py
27:from qiskit.providers.basicaer import BasicAerJob
197:                                               only works for Aer and BasicAer providers
341:            job = BasicAerJob(backend, job_id, backend._run_job, qobj)

aqua/utils/backend_utils.py
90:    """Detect whether or not backend is from BasicAer provider.
95:        bool: True is BasicAer
98:    from qiskit.providers.basicaer import BasicAerProvider
100:    return isinstance(backend.provider(), BasicAerProvider)

optimization/algorithms/minimum_eigen_optimizer.py
21:from qiskit import QuantumCircuit, BasicAer, execute
267:    result = execute(circuit, BasicAer.get_backend('statevector_simulator')).result()

aqua/components/initial_states/custom.py
119:        from qiskit import BasicAer
124:                    self._state_vector = np.asarray(q_execute(self._circuit, BasicAer.get_backend(

aqua/operators/expectations/expectation_factory.py
20:from qiskit import BasicAer
74:                        backend_to_check = BasicAer.get_backend('statevector_simulator')
80:                            'the BasicAer qasm backend for this expectation to avoid having to '
85:                        backend_to_check = BasicAer.get_backend('qasm_simulator')
92:            # If the user specified a statevector backend (either Aer or BasicAer),
103:            # All other backends, including IBMQ, BasicAer QASM, go here.

aqua/operators/primitive_ops/circuit_op.py
21:from qiskit import QuantumCircuit, BasicAer, execute
149:        unitary_backend = BasicAer.get_backend('unitary_simulator')

aqua/operators/state_fns/circuit_state_fn.py
21:from qiskit import QuantumCircuit, BasicAer, execute, ClassicalRegister
234:        statevector_backend = BasicAer.get_backend('statevector_simulator')
335:        qasm_backend = BasicAer.get_backend('qasm_simulator')

@t-imamichi
Copy link
Member Author

t-imamichi commented May 21, 2020

quantum_info.Operator's way is as follows. I think we can replace BasicAer in Aqua except quantum_instance by making utility functions, e.g., get_unitary_matrix(qc), get_statevector(qc), and get_count(qc).

What do you think?

from qiskit import QuantumCircuit
from qiskit.quantum_info import Operator, Statevector
from collections import Counter
from random import choices

qc = QuantumCircuit(2)
qc.h(0)
qc.cx(0,1)

samples=1000

zero = Statevector.from_int(0, 2**qc.num_qubits)
state = zero.evolve(qc)
print('unitary matrix:\n', Operator(qc).data)
print('state vector:', state.to_dict())
prob = state.probabilities_dict()
print('probability:', prob)
print('count:', Counter(choices(list(prob.keys()), list(prob.values()), k=samples)))
print()

# check endian

qc = QuantumCircuit(2)
qc.x(0)

zero = Statevector.from_int(0, 2**qc.num_qubits)
state = zero.evolve(qc)
print('unitary matrix:\n', Operator(qc).data)
print('state vector:', state.to_dict())
prob = state.probabilities_dict()
print('probability:', prob)
print('count:', Counter(choices(list(prob.keys()), list(prob.values()), k=samples)))

Output:

unitary matrix:
 [[ 0.70710678+0.j  0.70710678+0.j  0.        +0.j  0.        +0.j]
 [ 0.        +0.j  0.        +0.j  0.70710678+0.j -0.70710678+0.j]
 [ 0.        +0.j  0.        +0.j  0.70710678+0.j  0.70710678+0.j]
 [ 0.70710678+0.j -0.70710678+0.j  0.        +0.j  0.        +0.j]]
state vector: {'00': (0.7071067811865475+0j), '11': (0.7071067811865475+0j)}
probability: {'00': 0.4999999999999999, '11': 0.4999999999999999}
count: Counter({'00': 501, '11': 499})

unitary matrix:
 [[0.+0.j 1.+0.j 0.+0.j 0.+0.j]
 [1.+0.j 0.+0.j 0.+0.j 0.+0.j]
 [0.+0.j 0.+0.j 0.+0.j 1.+0.j]
 [0.+0.j 0.+0.j 1.+0.j 0.+0.j]]
state vector: {'01': (1+0j)}
probability: {'01': 1.0}
count: Counter({'01': 1000})

@t-imamichi
Copy link
Member Author

t-imamichi commented May 22, 2020

I tried to replace BasicAer in Aqua and found quantum_info.Operator does not handle reset and measure instructions. Some circuits generated in Aqua contains reset due to initializer.

We need to think how to deal with such circuits with BasicAer.

  1. Just use Aer instead of BasicAer
  2. Modify circuits not to include reset and measure

@woodsp-ibm
Copy link
Member

I was more thinking that QuantumInstance would support using QuantumInfo in the absence of any specific backend being passed in, in a way that allowed the algorithms to still perceive a counts based (qasm simulator) type output or statevector one. Algorithms already only include measurements when using backends such as qasm simulators or real devices. This would then allow any algo to run on Aer and or real backend and if none were provided would use QuantumInfo via the QuantumInstance (maybe with a flag on QuantumInstance to say counts versus statevector). This implicit use of QuantumInfo in QuantumInstance would thus replace the explicit passing of BasicAer as a backend since it will no longer exist.
For other components that directly used the BasicAer backend to access certain function then directly switch to QuantumInfo since they know what they are using.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 22, 2020

I think it's possible to fallback to QuantumInfo if no backend is specified. My PoC code to generate statevector and counts is simple. https://github.com/Qiskit/qiskit-aqua/blob/895d968371c45cea6e44f7a03e3beb8174bd8695/qiskit/aqua/utils/circuit_utils.py#L85
But, we have to deal with reset instruction included in Initialize. I'm not sure how it is difficult to simulate reset in QuantumInfo.

@woodsp-ibm
Copy link
Member

woodsp-ibm commented May 22, 2020

My take would be that we do not attempt to add extra logic to support function that is not there in QuantumInfo. If we can avoid using such specifics it might be good. But if not then we could either detect these and alert via an error, which may be unnecessary overhead, or just let them pass through which presumably will result in a different error. Depending on the error we might want to catch/augment it suggest use of Aer etc to run the algorithm in this case.
I would hope it was not too much code to support use of Quantum info - getting statevector or counts looked simple enough. Things just need to be transparent to the algo so it can, like it does already, pass a set of circuits to QuantumInstance and get the results back in a consistent way that it can handle them no matter what 'backend' is being used.

@t-imamichi
Copy link
Member Author

I agree. Extending QuantumInfo is not a good strategy. We have to discuss whether Aqua should depend on Aer or not. What do you think of including Aer in requirement.txt?

@woodsp-ibm
Copy link
Member

Internally in the couple of places where Aqua relies on doing some conversion can we not just always use QuantumInfo. For running an algo, when the user passes a QuantumInstance, if they explicitly give it a backend we would use that, otherwise QuantumInstance would use QuantumInfo (with a flag to choose counts/statevector). I do not see why we would need to include Aer as a hard dependency.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 25, 2020

@Cryoris
Copy link
Contributor

Cryoris commented May 25, 2020

I agree with Steve in the above, if we can avoid adding objects on our side then that would be great (and no hard Aer dependency).

As for reset: The initializers shouldn't use reset in the beginning I think, they are applied at the beginning of a circuit anyways. And there's currently a discussion going on to tell the circuit instance that it should start in zero. Is there another place where this is used?

Measurements could probably be taken care of inside the QuantumInstance: figure out which registers are measured and trace the rest out.

Another case we need to keep in mind, is that the operator cannot handle Barriers, but I think we can just remove them before simulation.

I think we should talk to Chris to see if that's how the Operator is intended to be used. In principle there are also non-unitary constructs such as the SuperOperator which can do reset, but they are more complex than just the Operator.

@woodsp-ibm
Copy link
Member

woodsp-ibm commented May 25, 2020

Perhaps the issue of the Initialize needs to be brought up then in the Terra Issue that is discussing the BasicAer removal. Clearly that extension will then not work with Terra alone as it is, and require Aer or some other backend having reset, unless something is done. Terra itself treats Aer as optional.

If it turns out that the circuit_state_fn has to end up dependent on Aer then we can always still treat Aer as an optional pkg and raise an exception that this logic cannot be used without Aer being installed.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 25, 2020

The comment of Initializer explains as follows. So, I think it is as intended.

Note that Initialize is an Instruction and not a Gate since it contains a reset instruction, which is not unitary.

https://github.com/Qiskit/qiskit-terra/blob/ad11f087dee21de3dd762647d2790b9ff7e3b63c/qiskit/extensions/quantum_initializer/initializer.py#L40

I found that circuit_state_fn is the only module that uses the initializer.

In my local env, test.aqua.operators.test_state_construction fails, but, there seems no failure in Travis. I'm wondering what is happening... (lint should fail because I didn't write docstrings yet)
https://github.com/Qiskit/qiskit-aqua/pull/1009/checks?check_run_id=706308592

@woodsp-ibm
Copy link
Member

In my local env, test.aqua.operators.test_state_construction fails, but, there seems no failure in Travis. I'm wondering what is happening...

Do you have a different version locally that you would expect it to fail the lint for you (lint being run via make lint of course). The version I see checked in (its not updated by the your PR you refer to) has docstring comment on each test function. The build is failing on circuit utils in lint though.

@t-imamichi
Copy link
Member Author

I remembered that transpile removes reset in initializer if possible. I tried it (qiskit-community/qiskit-aqua@83c90b1), but it results in bunch of unit test fails. I will look into them.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 26, 2020

I found that some gates are converted into another that is equal to the original one up to global phase, e.g., X -> U3(pi,0,pi), and some unit tests fail due to the global phase.
The following gates have this issue: ID, X, H, Hamiltonian.
I managed to pass all unit tests related to operators by disabling unrolling of these gates.
See ba7cde5

@t-imamichi
Copy link
Member Author

As a side effect, my PoC finishes unit tests faster than master.

master

$ python -m unittest test/aqua/operators/test_*.py
.......Measured Observable is not composed of only Paulis, converting to Pauli representation, which can be expensive.
.............Evolved Hamiltonian is not composed of only Paulis, converting to Pauli representation, which can be expensive.
..........................sMeasured Observable is not composed of only Paulis, converting to Pauli representation, which can be expensive.
....................
----------------------------------------------------------------------
Ran 67 tests in 14.738s

OK (skipped=1)

qiskit-community/qiskit-aqua#1009

$ python -m unittest test/aqua/operators/test_*.py
.......Measured Observable is not composed of only Paulis, converting to Pauli representation, which can be expensive.
.............Evolved Hamiltonian is not composed of only Paulis, converting to Pauli representation, which can be expensive.
..........................sMeasured Observable is not composed of only Paulis, converting to Pauli representation, which can be expensive.
....................
----------------------------------------------------------------------
Ran 67 tests in 6.983s

OK (skipped=1)

@Cryoris
Copy link
Contributor

Cryoris commented May 26, 2020

Just talked to Chris, the removal of the functionality of BasicAer.get_backend('qasm_simulator') is not definite yet, it might be kept because otherwise there's no easy way to include classical logic, measurement etc. with the quantum info module. If that's the case, and I think we are in a position to argue in favor of keeping it, that would make the switch a lot easier.

Note: that just concerns the qasm_simulator, not statevector or unitary.

@t-imamichi
Copy link
Member Author

It's a good news. As far as Terra keeps BasicAer, it would be nice keep BasicAer in Aqua too.
But, quantum_info-based replacement is almost ready. We don't need to worry it for a while.

What do you mean "just concerns the qasm_simulator"? It is easy to generate counts from statevector.probabilities_dict.

@Cryoris
Copy link
Contributor

Cryoris commented May 27, 2020

E.g. how would you run this with quantum info?

        ┌───┐┌─┐ ┌───┐
    q0: ┤ H ├┤M├─┤ X ├─
        └───┘└╥┘ └─┬─┘
              ║ ┌──┴──┐
    c0: ══════╩═╡ = 1 ╞
                └─────┘

The main restriction of the Statevector object is that you cannot handle all instruction Qiskit offers, but the Aer/BasicAer simulators can.

@t-imamichi
Copy link
Member Author

t-imamichi commented May 27, 2020

You are right. Statevector and Operator cannot deal with conditionals. Are such circuits used in Aqua? I think CircuitLibrary contains such circuits maybe, right?

@woodsp-ibm
Copy link
Member

We had not used conditionals in circuits we created in Aqua in the past. Unless any circuits had these introduced when they became part of the library, from where we now use these, and I do not think they did, I do not think we use these.

@Cryoris
Copy link
Contributor

Cryoris commented May 27, 2020

There were no conditionals introduced with the move to the library, so I think no algorithm currently uses them. But in future there might be algos that do, so we probably shouldn't add any restrictions on what can be run or not

@woodsp-ibm
Copy link
Member

If algos choose to use this in the future it would need to be based on whether the backend supports them or not, as appropriate, since conditional is presently not supported by actual devices. So in that regard there are restrictions that need to be catered to.

@t-imamichi
Copy link
Member Author

Thank you for all information. As far as real devices don't support conditionals, Aqua don't have to support it either. Otherwise, such quantum algorithms can run with only Aer/BasicAer.

I think of the following strategy.

  1. While Terra maintains BasicAer, Aqua continues to use it.
  2. When Terra removes BasicAer, we will discuss again so that we replace BasicAer with Aer or Statevector (Operator).

So, how about putting this issue on hold?

@Cryoris
Copy link
Contributor

Cryoris commented May 28, 2020

As far as real devices don't support conditionals, Aqua don't have to support it either.

I don't agree. Only because our hardware isn't quite there yet that doesn't mean we cannot test more advanced quantum algorithms. After all, we've been testing large qubit number and circuits w/o decoherence, which hardware doesn't support (yet) 😉
After all it's about testing what quantum algorithms could do.

We can always tell users that this won't yet run on actual hardware -- which happens right now already if you try to run conditionals.

@Cryoris
Copy link
Contributor

Cryoris commented May 28, 2020

The strategy sounds good. As such we have a plan ready, if we need it.

@woodsp-ibm
Copy link
Member

So, how about putting this issue on hold?
Sure, we do not need to act until we know it will be removed. It was good to get an understanding of what this might entail so that we can add to the discussion.

In terms of support of various backend capabilities (conditionals or whatever) ideally the algorithms should be built to run and leverage, potentially optionally, any capabilities that would improve their performance/behavior but arguably not at the expense of limiting their ability to run on any backend including real devices where people want to try them out albeit on smaller problems that may fit. Algos currently adapt to qasm or statevector modes and the goal has been to made algorithms more hardware (backend) aware/adaptable in order to improve their outcome.

@Cryoris
Copy link
Contributor

Cryoris commented May 28, 2020

Yes, the implementations of the algorithms in Aqua should be able to run on hardware, if possible. We shouldn't introduce artificial limitations by default (like adding conditionals). But if users come up with an algorithm that uses these features that we are able to simulate and will be able to run in future, I don't see why we should disallow this.

@woodsp-ibm
Copy link
Member

I am not saying to disallow as such. More that it would be preferable if such an algorithm would optionally run, in some alternate manner, such that one can target more backends. Maybe it runs less optimally. If it were such that even for small problems some alternate mode is unrealistic then it maybe that it could only run on certain backends until some future time. I would not see us rejecting such an algo, just preferring that it be more adaptable until that point if possible.

@woodsp-ibm woodsp-ibm transferred this issue from qiskit-community/qiskit-aqua Jan 26, 2021
@woodsp-ibm woodsp-ibm added the mod: algorithms Related to the Algorithms module label Jan 26, 2021
@woodsp-ibm
Copy link
Member

Algorithms have been refactored over to use primitives - this is no longer relevant at all. As such I am closing this issue.

@woodsp-ibm woodsp-ibm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: algorithms Related to the Algorithms module
Projects
None yet
Development

No branches or pull requests

3 participants