Change dtype repr for Numpy 1.24 #5991
Conversation
pavoljuhas
added
the
triage/accepted
Temporarily upgrade to numpy-1.24.2 to enable CI testing of quantumlib#5991. Also add an intentionally failing CI check to ensure this commit is not accidentally merged. Please revert this commit before merging the pull-request.
Temporarily upgrade to numpy-1.24.2 to enable CI testing of quantumlib#5991. Restrict CI-tests to Python 3.8 only which permits installation of numpy-1.24.2 and numba. Note numba is not yet compatible with numpy-1.24 and its import raises SystemError exception SystemError: initialization of _internal failed without raising an exception Here we also add intentionally failing CI check to ensure this commit is not accidentally merged to master. Please revert this commit before merging the pull-request.
|
Thank you @bnavigator for contributing this. Could you please take care of the lint / formatting CI errors above and the remaining I'd suggest to cherry-pick or re-apply 5d0e6d9 from my temporary PR #6028, which converts numpy deprecation warnings to test errors. Once this is passing the CI and merged, we can be fairly sure old attributes won't be reintroduced. Were there any other test failures you saw in the |
There was a problem hiding this comment.
Thank you for contributing this. Please see my suggestions in #5991 (comment)
bnavigator
force-pushed
the
core-np1.24
branch
2 times, most recently
from
e7433ab to
b84d1bd
Compare
| @@ -1 +1 @@ | |||
| cirq_google.EngineResult(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[True, True, False, True, False]], [[False, True, True, False, False]], [[True, False, True, False, True]]], dtype=np.bool)}, job_id='my_job_id', job_finished_time=datetime.datetime(2022, 4, 1, 8, 23, 45, tzinfo=datetime.timezone.utc)) | |||
| cirq_google.EngineResult(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[True, True, False, True, False]], [[False, True, True, False, False]], [[True, False, True, False, True]]], dtype=bool)}, job_id='my_job_id', job_finished_time=datetime.datetime(2022, 4, 1, 8, 23, 45, tzinfo=datetime.timezone.utc)) | |||
There was a problem hiding this comment.
This repr is not what gets generated if I call repr(engine) where engine is the corresponding object, right?
For example, this is what I get when I run repr(engine) using your PR, where dtype=np.dtype('bool') instead of dtype=bool.
>>> repr(engine)
cirq_google.EngineResult(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[True, True, False, True, False]], [[False, True, True, False, False]], [[True, False, True, False, True]]], dtype=np.dtype('bool'))}, job_id='my_job_id', job_finished_time=datetime.datetime(2022, 4, 1, 8, 23, 45, tzinfo=datetime.timezone.utc))In general, all the .repr files should contain the output of calling repr(val) for each val. Since we are changing the repr generation, we'd need to update every object that is impacted by this change. i.e. Let Object be a class s.t. repr(object) used to contain np.int and now contains np.dtype(int). We need to ensure that Object.repr is updated to reflect the new output of repr(object).
Also fail when attributes are used in eval expressions. This prepares for upgrade to NumPy 1.24 which removes deprecated names.
| @@ -1 +1 @@ | |||
| [cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[True, True, False, True, False]], [[False, True, True, False, False]], [[True, False, True, False, True]]], dtype=bool)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[1, 1, 0, 1, 0]], [[0, 1, 1, 0, 0]], [[1, 0, 1, 0, 1]]], dtype=np.uint8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int16)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int32)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int64)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint16)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint32)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint64)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[1, 1, 0, 1, 0]], [[0, 1, 1, 0, 0]], [[1, 0, 1, 0, 1]]], dtype=np.uint8), 'n': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int64)})] | |||
| [cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[True, True, False, True, False]], [[False, True, True, False, False]], [[True, False, True, False, True]]], dtype=np.dtype('bool'))}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[1, 1, 0, 1, 0]], [[0, 1, 1, 0, 0]], [[1, 0, 1, 0, 1]]], dtype=np.uint8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int16)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int32)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int64)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint8)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint16)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint32)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.uint64)}), cirq.ResultDict(params=cirq.ParamResolver({sympy.Symbol('a'): 0.5}), records={'m': np.array([[[1, 1, 0, 1, 0]], [[0, 1, 1, 0, 0]], [[1, 0, 1, 0, 1]]], dtype=np.uint8), 'n': np.array([[[0, 1, 2]], [[3, 4, 5]], [[6, 7, 8]]], dtype=np.int64)})] | |||
There was a problem hiding this comment.
Were these regenerated using repr(object) ? It looks like dtype=np.uint64 should also be dtype=np.dtype('uint32') instead ?
I rebased the only relevant changes on top of the current master. This is a perfectly legit pattern in git. All the relevant history is still there. Side note: Do you have any idea how annoying it is to have to change a courtesy contribution from 2 months ago because you add more and more demands? |
|
I understand, this doesn't need to be blocking so I've opened a separate issue to track this change. #6049 Thank you for the patience and the courtesy contribution! We can merge once the CI passes. |
* Change numpy dtype repr * Enforce deprecation of np attributes deprecated in NumPy 1.20 Also fail when attributes are used in eval expressions. This prepares for upgrade to NumPy 1.24 which removes deprecated names. * update repr test data for numpy dtypes * break string not found by black * fix dumb typo --------- Co-authored-by: Pavol Juhas <[email protected]> Co-authored-by: Tanuj Khattar <[email protected]>
This is a step towards #5967. It uses the direct repr string from numpy itself
dtype('typename')but with the prefixnp., which should work for Python-builtins like 'int','float','bool' as well as numpy-specific types.With this patch, the rpm package for python-cirq 1.1.0 on openSUSE Tumbleweed passes. We need it because we patched numba with numba/numba#8620 and have NumPy 1.24 already.
One part still missing from #5967, which is irrelevant for the openSUSE package, are the json validations. We have been disabling those with pytest parameter
-k "not (test_json_test_data_coverage or test_json_and_repr_data)"because they don't like our installation prefix.