QuantumCircuit has no attribute 'save_statevector'

[teaguetomesh]

Information

• Qiskit Terra version: 0.17.1
• Python version: 3.7.9
• Operating system: macOS Big Sur 11.3

What is the current behavior?

Unable to apply the save_statevector instruction to a newly created QuantumCircuit. Doing so throws an error: AttributeError: 'QuantumCircuit' object has no attribute 'save_statevector'

However, if I first simulate a circuit without the save_statevector instruction, then everything works as intended and I can go back and apply the save_statevector instruction.

Steps to reproduce the problem

from qiskit import QuantumCircuit
circ = qk.QuantumCircuit(2)
circ.h(circ.qubits)
circ.save_statevector()

What is the expected behavior?

I should be able to apply the save_statevector instruction to my quantum circuit.

[nonhermitian]

Yes this is confusing as these are Aer instructions that are added to the QuantumCircuit class when Aer is imported, but show up under docs as being part of the main QuantumCircuit API, i.e. it shows up under Terra docs:

https://qiskit.org/documentation/stubs/qiskit.circuit.QuantumCircuit.html#qiskit.circuit.QuantumCircuit.save_statevector

[yaelbh]

Just importing Aer does not load the instructions. You need to explicitly call Aer.get_backend(...) for them to be available.

[nonhermitian]

Either way, it is confusing as they appear as native methods of QuantumCircuit in the documentation.

[WiFisunset]

@nonhermitian I agree. There are 2 separate solutions I can think of.

The first would be to change the tutorial documentation to suggest calling the simulator first.

The second, would be to change qc.save_unitary() to simulator.save_unitary() or simulator.save_unitary(qc).

Would moving save_unitary() and other save states to the class containing the Simulators raise any issues is the question (I'll take a look at the code later when I can get to it).

[yaelbh]

The solution is to change Terra code such that qc.save_unitary() is possible anytime, regardless of whether the simulator was called. An error will be invoked if one tries to run or transpile the circuit with a backend that doesn't support the save_unitary instruction.

[WiFisunset]

@yaelbh I took a look at the Aer instructions library (https://qiskit.org/documentation/apidoc/aer_library.html) and the documentation everywhere is saying that a simulator call needs to be made.

So save_unitary() is completely dependent on a simulator being called, as is the same for save statevector and etc. Which makes me think any fix to where we don't have to call a simulator to use save_unitary, would involve adding a simulator call to the instructions library or one of the Classes containing the save states (ex: save_unitary(param1, param2, param3, "simulator-of-choice")).

The only documentation that differs in not specifying that a simulator is needed first is the tutorial code. Updating the documentation might be a good temp fix at the very least.

(The below image is the init description for save_unitary):

[yaelbh]

Could you please refer more specifically where in https://qiskit.org/documentation/apidoc/aer_library.html it says that a simulator call is required? I see that it says that Aer needs to be imported, but in practice we see that merely from qiskit import Aer is not enough.

[WiFisunset]

Sure thing. I took a screenshot of the save state description. Specifically the line "can be used to save the state of the Simulator".

Could you please refer more specifically where in https://qiskit.org/documentation/apidoc/aer_library.html it says that a simulator call is required? I see that it says that Aer needs to be imported, but in practice we see that merely from qiskit import Aer is not enough.

[WiFisunset]

@yaelbh also more specifically, if a simulator call didn't need to be made, then "pershot" wouldn't be a parameter for save_unitary()

[yaelbh]

I don't see how the line "can be used to save the state of the Simulator" implies that the code crashes if Aer.get_backend(...) is not called first. All these marked lines only say that these instructions belong to the simulator. They don't say that a certain command line or simulation execution is required first.

[yaelbh]

Same for pershot: yes, everything is related to the simulator only. Still there is no reason not to be able to build a circuit with these instructions, especially if Aer was imported. A call to get_backend is not a declaration "Attention! We are using a simulator, allow simulator instructions from now on!".

[WiFisunset]

@yaelbh I see what you mean, but Python as a language is line-by-line execution. So if the line save_unitary(pershot=True) were called, then Python is expecting a simulator when executing that line.

So when 'get_backend' is called before the save state, python knows what to execute pershot due to 'get_backend' being executed previously. That may be the issue (the way python executes the code).

Same for pershot: yes, everything is related to the simulator only. Still there is no reason not to be able to build a circuit with these instructions, especially if Aer was imported. A call to get_backend is not a declaration "Attention! We are using a simulator, allow simulator instructions from now on!".

[mtreinish]

The solution is to change Terra code such that qc.save_unitary() is possible anytime, regardless of whether the simulator was called. An error will be invoked if one tries to run or transpile the circuit with a backend that doesn't support the save_unitary instruction.

I disagree with this, the semantics of the save state instructions are very specific to aer, and don't apply to simulators more broadly (as they all work differently). Instructions do not need to live in terra, unless we want them to be a common interface for everything.

As for the import error I think the point of confusion is that:

from qiskit import Aer

does not actually import qiskit-aer the package which means the monkey patching that aer does on import is not run. qiskit.Aer is a lazy loading alias for qiskit.providers.aer.Aer. Just importing qiskit.Aerdoesn't actually import the aer package just that wrapper. This is why running qiskit.Aer.get_backend() works because the wrapper will import qiskit.providers.aer.Aer under the covers to alias the call to get_backend().

As @nonhermitian mentioned it's an unfortunate consequence of the use of namespace packaging combined with how we build the combined documentation for the metapackage that the monkey patching that aer does to add the circuit methods happens before the docs are built, so they show up as if they're actually methods of the QuantumCircuit class.

I believe the fix here is simply to modify the docstrings for the save_* instruction functions to highlight qiskit.providers.aer must be imported for the instructions to work. Ideally in the summary line so it shows up in the methods table too.

[yaelbh]

@mtreinish I agree that the fix for now is to document the need to import qiskit.providers.aer. Still in the long term we should further fix get_backend. This is a query, and as such, according to the command-query separation principle, should not do anything but returning the backend. It is allowed, internally, to perform lazy evaluations, as long as they are not exposed to the user, which is not the case here.

[WiFisunset]

does not actually import qiskit-aer the package which means the monkey patching that aer does on import is not run. qiskit.Aer is a lazy loading alias for qiskit.providers.aer.Aer. Just importing qiskit.Aerdoesn't actually import the aer package just that wrapper. This is why running qiskit.Aer.get_backend() works because the wrapper will import qiskit.providers.aer.Aer under the covers to alias the call to get_backend().

I believe the fix here is simply to modify the docstrings for the save_* instruction functions to highlight qiskit.providers.aer must be imported for the instructions to work. Ideally in the summary line so it shows up in the methods table too.

I ran the code as intended, but this time imported via (from qiskit.providers.aer import Aer), and the code works. I'll update my pull request to reflect that in fix Qiskit/qiskit-tutorials#1176.

@yaelbh I've been thinking about the issue you're raising, and trying to find a solution for it. So to start I looked at the following to see how any change would affect get_backend (https://qiskit.org/documentation/stubs/qiskit.providers.aer.AerProvider.html#qiskit.providers.aer.AerProvider.get_backend and also, https://qiskit.org/documentation/stubs/qiskit.providers.Backend.html#qiskit.providers.Backend).

And it seems get_backend accepts keyword or named arguments through **kwargs, and then returns the backend which is then initialized (based off of what named arguement for the backend was requested; ex: 'unitary_simulator'). So get backends job seems to be 2 things, the first is to take a name argument to determine what simulator to initialize, and then to return that initialized backend. I'm not sure if the two tasks can or should be separated though (I'm not sure what that would impact overall).

[rht]

I second the second solution proposed by @WiFisunset

The second, would be to change qc.save_unitary() to simulator.save_unitary() or simulator.save_unitary(qc).

My reasoning is that this is an operation specific to the Aer backend, and so should be localized to it. You can't always rely on people to read the whole documentation, and so a targeted error message is more effective and user-friendly.

[rht]

This is another possible solution: rht@27d934f, i.e. inform user in the error message that they should initialize the Aer backend.

If you think this is a good fix (for now), I can add test to it and make a PR.

[HuangJunye]

Based on the discussions above and #8359 (comment), it sounds like this is not something we should fix in Terra. Should we close this issue as "Won't fix"? @jakelishman

[HuangJunye]

Reading a bit more carefully, the suggested solution is to fix the docstrings #6346 (comment)

I believe the fix here is simply to modify the docstrings for the save_* instruction functions to highlight qiskit.providers.aer must be imported for the instructions to work. Ideally in the summary line so it shows up in the methods table too.

I think this should apply to set_* instruction functions as well. This is an related issue on that: https://github.com/Qiskit/qiskit/issues/1578

[jakelishman]

We can leave this open to refer to until we're got the documentation fix in Aer, but yeah, this is something that wants fixing there, not in Terra.

[HuangJunye]

We had an offline discussion about this. This issue should be fixed after qiskit-aer move to its own namespace.

[mforbes]

Perspectives from a confused user: I am trying to use QuantumCircuit for teaching, so I would like to be able to create a circuit, then display it, using save_statevector() called at various places to represent visually in the circuit diagram where these statevectors will be saved (once #8440 makes it into a wheel). This is all done long before thinking about importing a simulator.

I view the .save_statevector() as a promise that later, when I simulate the circuit with the appropriate simulator, I will be able to get a copy of this (which I can print out for students to check their work). From this perspective, it makes sense to keep it as a part of the QuantumCircuit class and it is highly counter-intuitive that one would need to import a simulator or get a backend (without any reference to the circuit in question) in order to be able to call this method.

[jakelishman]

We really consider save_statevector to be a property of a simulator, not part of a description of a circuit, which is why we're moving to take it off QuantumCircuit entirely. The semantics of such savings are often very simulator-dependent, which is why the instruction's defined in Aer only, and other simulators of quantum circuits might define their own. Not all Aer simulation methods come close to supporting this, for illustration - the extended stabiliser, matrix-product state, density-matrix, unitary evolution, etc etc simulators all don't handle this, and simulator directives aren't something we can "basis translate" away into something more meaningful (unlike not-directly-supported quantum gates).

I feel like this would be clearer if the method had never been attached to QuantumCircuit at all. The fact that it is is a historical quirk of how originally all instructions were magically monkey-patched onto QuantumCircuit, and we only centralised them later. It also doesn't help that Aer used to be part of the "Qiskit" namespace package that's owned by Terra (very confusing), which we've just disentangled - going forwards, Aer will be imported with import qiskit_aer (though the old imports will continue to work for a while to ease the transition).

We will still support adding such instructions to QuantumCircuit, but the plan is that you'll need to import them directly from qiskit_aer, and then add them with QuantumCircuit.append, to properly highlight how they're simulator-specific.

(fyi: #8440 should be in a wheel in two weeks' time - the planned release date for Terra 0.22 is two weeks from today, which will include that PR.)

[mforbes]

Thanks. Could you point me to some docs that show me what this will look like? Currently I use something like

import qiskit
qc = qiskit.QuantumCircuit(2)
qc.save_statevector(label=r"\psi_0")
qc.h(0)
qc.save_statevector(label=r"\psi_1")
qc.cx(0,1)
qc.save_statevector(label=r"\psi_2")
qc.draw('mpl')

Would this become something like

import qiskit, qiskit_aer
sim = qiskit_aer.StatevectorSimulator()
qc = qiskit.QuantumCircuit(2)
qc.append(sim.save_statevector(label=r"\psi_0"))  # Does not exist... what goes here?
qc.h(0)
...

[jakelishman]

We don't have any docs yet, sorry - the action item from this issue is still about just documenting the current behaviour, which we're behind on!

For now, the right thing to do is just to import qiskit.providers.aer at the top of your file underneath the import qiskit line (or import qiskit_aer if you can guarantee that everyone will be using the latest Aer 0.11). Then you can just use the circuit method save_statevector like you have been doing.

We do need to update all this documentation fairly urgently - the community team are currently in the process of trying to bring some order to all our documentation, and the core Terra team are currently a bit overwhelmed with the upcoming 0.22 release, which is why things are moving particularly slowly right now (though this issue is so old, we don't really have an excuse...). I'm sorry it's confusing in the mean time.

[mforbes]

Thanks, that will fix my issues. However, I would really recommend against having methods like save_statevector pop up based on imports... this is very confusing from a user perspective, even if well documented. Many people use introspection to see what objects can do, sometimes without fully reading the docs. I feel it should either be there at the beginning, inserting some sort of tag that appropriate simulators can use, or should explicitly require an appropriate simulator to call.

[jakelishman]

Yeah, I totally agree - I want to move to that model (explicitly require an appropriate simulator) as well. We're going in that direction, but development on Aer's interface is quite slow, and we need to have a good long deprecation period to ease the transition for everyone. It's just in the interim where "improve the documentation" is important, while we do the deprecations.

[jakelishman]

With qiskit_aer having its own totally separate documentation that lists the added circuit methods, I think we can close this issue. The question still remains as to whether Aer should patch QuantumCircuit, but that would be an issue to raise on Aer.