Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Consistent behaviour of StateFn.eval with OperatorBase.eval #1210

Merged
merged 16 commits into from
Aug 24, 2020

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Aug 20, 2020

Summary

Allow calling eval on state function objects with no argument, which returns the VectorStateFn representation of the state function. This is consistent behaviour with OperatorBase.eval, which returns the MatrixOp representation, if no argument is passed.

Details and comments

All state functions are converted to the VectorStateFn with the existing method to_matrix_op. For the OperatorStateFns it is assumed that the operator acts on the zero state (as is assumed in the CircuitStateFn if a circuit is passed), therefore the first row is returned as vector.

@Cryoris Cryoris added the Changelog: New Feature Include in the Added section of the changelog label Aug 20, 2020
@Cryoris Cryoris added this to the 0.8 milestone Aug 20, 2020
@Cryoris Cryoris requested a review from Zoufalc August 20, 2020 10:24
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow calling eval on state function objects with no argument, which returns the VectorStateFn representation of the state function.

As the eval here inherits its docstring from OperatorBase can that be updated to describe this change. It talks about return values when self is a StateFn

This is consistent behaviour with OperatorBase.eval, which returns the MatrixOp representation, if no argument is passed.

OperatorBase eval is abstract and I do not see any such definition in OperatorBase eval docstring that this should be the behavior of subclasses when front is None. Is this the case elsewhere, maybe the docstring should be improved in this regard too

@@ -277,6 +277,11 @@ def assign_parameters(self, param_dict: dict) -> OperatorBase:
def eval(self,
front: Union[str, dict, np.ndarray,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why, unlike OperatorBase, this has never been typehinted as Optional. Clearly it defaulted to None so should have been. Can we change this in all these state fn signatures especially now since it seem it will work in all cases whereas before it looks like things may well have failed in some of these should None have actually been passed.

Also I note the signature in OperatorBase for eval is a bit more specific around the Dict type on the eval signature than was done in the typehints in these state fns.

Now circuit_state_fn, if I am reading the code correctly would already drop through and do what is coded here anyway as the default last line. Maybe you want this explicit code here so its consistent with the other state fns.

@Cryoris
Copy link
Contributor Author

Cryoris commented Aug 22, 2020

Good point 👍 I added the optional typehint and put Dict[str, complex] instead of just dict in a937d3e

@woodsp-ibm woodsp-ibm merged commit 72a6fb3 into qiskit-community:master Aug 24, 2020
Cryoris added a commit to Cryoris/qiskit-aqua that referenced this pull request Sep 3, 2020
…ommunity#1210)

* eval on a statefn returns the sampling probs

* add tests, rm print

* add reno

* return VectorStateFn instead of Statevector

* Change test case to use ddt instead of subtest loop

Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
pbark pushed a commit to pbark/qiskit-aqua that referenced this pull request Sep 16, 2020
…ommunity#1210)

* eval on a statefn returns the sampling probs

* add tests, rm print

* add reno

* return VectorStateFn instead of Statevector

* Change test case to use ddt instead of subtest loop

Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
mtreinish pushed a commit to mtreinish/qiskit-core that referenced this pull request Nov 20, 2020
…ommunity/qiskit-aqua#1210)

* eval on a statefn returns the sampling probs

* add tests, rm print

* add reno

* return VectorStateFn instead of Statevector

* Change test case to use ddt instead of subtest loop

Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 2, 2020
…ommunity/qiskit-aqua#1210)

* eval on a statefn returns the sampling probs

* add tests, rm print

* add reno

* return VectorStateFn instead of Statevector

* Change test case to use ddt instead of subtest loop

Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
manoelmarques added a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 7, 2020
…ommunity/qiskit-aqua#1210)

* eval on a statefn returns the sampling probs

* add tests, rm print

* add reno

* return VectorStateFn instead of Statevector

* Change test case to use ddt instead of subtest loop

Co-authored-by: Manoel Marques <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: woodsp <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants