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

Expectation behavior of factory #5746

Closed
woodsp-ibm opened this issue Aug 1, 2020 · 1 comment
Closed

Expectation behavior of factory #5746

woodsp-ibm opened this issue Aug 1, 2020 · 1 comment
Labels
mod: algorithms Related to the Algorithms module

Comments

@woodsp-ibm
Copy link
Member

The expectation factory checks the operator and will return a valid expectation if the operator is all paulis or matrices. However for a mix the following happens raise ValueError('Expectations of Mixed Operators not yet supported.')

This test below was in test_qaoa - it has been updated/changed as part of qiskit-community/qiskit-aqua#1154 which ensures, when the expectation as supplied was None, and VQE should auto-select a valid expectation, that indeed this happens. specifically around re-use when backend and/or operator can be changed. Before the PR the very first expectation auto-selected by VQE ended up 'locked-in' since it simply checked an instance var where it stored it for None. This meant switching the backend or operator may fail to have the best, or even an expectation that simply worked with the new backend and/or operator.

        """ QAOA test """
        backend = BasicAer.get_backend('statevector_simulator')
        optimizer = COBYLA(maxiter=2)
        qubit_op, _ = max_cut.get_operator(W1)
        qubit_op = qubit_op.to_opflow().to_matrix_op()

        seed = 0
        aqua_globals.random_seed = seed
        qaoa = QAOA(qubit_op, optimizer, P1)
        quantum_instance = QuantumInstance(backend, seed_simulator=seed, seed_transpiler=seed)
        qaoa.run(quantum_instance)
        qaoa.operator = (X ^ qubit_op ^ Z)
        qaoa.run()

Here is sets a qubit_op the first time around as computed by qubit_op.to_opflow().to_matrix_op() which ends up with a MatrixOperator.Before qiskit-community/qiskit-aqua#1154 the next operator (a TensoredOp) passed (X ^ qubit_op ^ Z) would not cause an issue. However if it were passed in as the first one then the warning about Mixed Operators is raised. It seems that having the MatrixOperator 'locked-in' from the first operator it was able to at least not fail. The result was never checked and I do not know if things work as expected or not. Following qiskit-community/qiskit-aqua#1154 the change of the operator will cause it to autoselect an expectation via the factory and it will raise the error whether passed in first or second in the reuse test as above. (If MatrixExpectation instance is explicitly passed, meaning that there is no longer any auto-selection and VQE will used that until such time, if ever, its changed, then the TensoredOp is accepted).

Since there seems to be some difference in here in behavior around the factory which claims the expectation of mixed operators is not supported, and the fact that its accepted if a MatrixOperator is provided which seems to infer some under the covers conversion is happening I think this need investigating and improving in some way. Maybe the factory message should be more that the factory does not determine the most efficient way to convert mixed operators to determine the right expectation object. Thats a bit less blunt than expectation of mixed operators is not supported. Either way I think there is room for improvement here.

@woodsp-ibm woodsp-ibm transferred this issue from qiskit-community/qiskit-aqua Jan 29, 2021
@woodsp-ibm woodsp-ibm added the mod: algorithms Related to the Algorithms module label Jan 29, 2021
Cryoris added a commit to Cryoris/qiskit-terra that referenced this issue Jun 15, 2021
mergify bot added a commit that referenced this issue Jun 17, 2021
* make VQE stateless

TODO deprecation

* fix get initial point from ansatz

* try fixing use of deprecate_function

* fix tests & black

* deprecate varalgo methods

* sort params per circuit default

* clarify deprecation message in varalgo.initialpoint

* initialize eval_count with 0 instead of None

it doesn't really matter, having it None was more a safeguard, but adaptVQE breaks if this is None and not 0

* simplify get_eigenstate

* fix lint

* remote note on parameter order

* review suggestions

* deprecate sort_parameters_by_name
* check ansatz has 0 params in setter
* rename varform -> ansatz

* fix tests

* lint

* keep previous behaviour of expectation

meaning that if a user didn't set it, we re-construct the expectation for each new run of VQE

* move expectation factory to right place

* address #5746

* fix typehint in expectation (add Optional)

* fix expectation setter

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@woodsp-ibm
Copy link
Member Author

Algorithms have been refactored over to use primitives and observable estimation (expectations) are now handled by these. 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
ElePT pushed a commit to ElePT/qiskit that referenced this issue Jun 27, 2023
* make VQE stateless

TODO deprecation

* fix get initial point from ansatz

* try fixing use of deprecate_function

* fix tests & black

* deprecate varalgo methods

* sort params per circuit default

* clarify deprecation message in varalgo.initialpoint

* initialize eval_count with 0 instead of None

it doesn't really matter, having it None was more a safeguard, but adaptVQE breaks if this is None and not 0

* simplify get_eigenstate

* fix lint

* remote note on parameter order

* review suggestions

* deprecate sort_parameters_by_name
* check ansatz has 0 params in setter
* rename varform -> ansatz

* fix tests

* lint

* keep previous behaviour of expectation

meaning that if a user didn't set it, we re-construct the expectation for each new run of VQE

* move expectation factory to right place

* address Qiskit#5746

* fix typehint in expectation (add Optional)

* fix expectation setter

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this issue Jul 17, 2023
* make VQE stateless

TODO deprecation

* fix get initial point from ansatz

* try fixing use of deprecate_function

* fix tests & black

* deprecate varalgo methods

* sort params per circuit default

* clarify deprecation message in varalgo.initialpoint

* initialize eval_count with 0 instead of None

it doesn't really matter, having it None was more a safeguard, but adaptVQE breaks if this is None and not 0

* simplify get_eigenstate

* fix lint

* remote note on parameter order

* review suggestions

* deprecate sort_parameters_by_name
* check ansatz has 0 params in setter
* rename varform -> ansatz

* fix tests

* lint

* keep previous behaviour of expectation

meaning that if a user didn't set it, we re-construct the expectation for each new run of VQE

* move expectation factory to right place

* address Qiskit/qiskit#5746

* fix typehint in expectation (add Optional)

* fix expectation setter

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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

1 participant