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

cirq.Result type validation for measurement argument #3712

Open
balopat opened this issue Jan 27, 2021 · 2 comments
Open

cirq.Result type validation for measurement argument #3712

balopat opened this issue Jan 27, 2021 · 2 comments
Labels
area/measurements area/mypy area/result kind/health For CI/testing/release process/refactoring/technical debt items

Comments

@balopat
Copy link
Contributor

balopat commented Jan 27, 2021

cirq.Result is supposed to be supplied measurement results as a dict from string to a multidimensional numpy array. However it is possible to instead supply the value as a list of lists. In this case cirq.Result equality will still work, but it will fail to when converted to a string (since __str__ uses numpy slicing).

I'm not quite sure why mypy didn't yell about the incorrect type here. I guess the value in a dict is covariant? I thought it was invariant.

mypy doesn't recognize numpy types.
All of this is accepted by default

Result(params=ParamResolver({}), measurements={"a":[1,2,3]}) # should fail but it won't 
Result(params=ParamResolver({}), measurements={"a":"b"})  # should fail but it won't 
Result(params=ParamResolver({}), measurements={"a":[[1,0],[0,1]]})  # should fail but it won't 

Result(params=ParamResolver({}), measurements={"a":np.array([1,2,3])})  # OK 
Result(params=ParamResolver({}), measurements={"a":np.array([[1,0],[0,1]])}) # OK 

I tried with pip install data-science-types - that project creates type stubs for numpy, matplotlib, pandas.
With this, mypy catches the first three:

cirq/bla.py:5: error: Dict entry 0 has incompatible type "str": "List[int]"; expected "str": "ndarray[Any]"
cirq/bla.py:6: error: Dict entry 0 has incompatible type "str": "str"; expected "str": "ndarray[Any]"
cirq/bla.py:7: error: Dict entry 0 has incompatible type "str": "List[List[int]]"; expected "str": "ndarray[Any]"

However, it generates a lot of other noise:

Found 1337 errors in 154 files (checked 763 source files)

That includes errors due to missing type stubs in the package e.g.:

cirq/contrib/acquaintance/gates_test.py:20: error: Module 'numpy.random' has no attribute 'poisson'
cirq/contrib/acquaintance/executor_test.py:93: error: Module has no attribute "allclose"
cirq/contrib/acquaintance/executor_test.py:94: error: Module has no attribute "absolute"
cirq/contrib/acquaintance/executor_test.py:116: error: Module has no attribute "random"; maybe "rand"?
cirq/contrib/acquaintance/executor_test.py:177: error: Unexpected keyword argument "verbose" for "assert_allclose"

So I think we'll just have to live with this not being in mypy. Maybe we could / should do runtime type validation in the constructor though for the measurement argument!

Originally posted by @balopat in #3710 (comment)

@balopat balopat added area/measurements area/result kind/health For CI/testing/release process/refactoring/technical debt items labels Jan 29, 2021
@95-martin-orion
Copy link
Collaborator

Related to #3767.

@dstrain115
Copy link
Collaborator

We can probably close this after numpy requirements go past 1.20 where they have mypy stubs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/measurements area/mypy area/result kind/health For CI/testing/release process/refactoring/technical debt items
Projects
Status: No status
Development

No branches or pull requests

5 participants