Modify some numpy types to be compatible with `numpy>=1.20` #5668

vtomole · 2022-07-06T21:54:10Z

Part of #3767

dstrain115

One comment, otherwise seems fine to me.

dstrain115 · 2022-07-06T22:29:30Z

cirq-core/cirq/qis/clifford_tableau.py

@@ -583,6 +583,6 @@ def apply_global_phase(self, coefficient: linear_dict.Scalar):
        pass

    def measure(
-        self, axes: Sequence[int], seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None
+        self, axes: Sequence[int], seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE'


Are we sure that removing the default won't break anything? Is it possible anyone is calling measure() without specifying a seed? (Same for below).

I've put the default back but set it to np.random instead of None because it's needed by _measure.

pavoljuhas · 2022-07-06T23:24:39Z

cirq-core/cirq/circuits/circuit.py

@@ -2588,7 +2589,7 @@ def _apply_unitary_circuit(
    state: np.ndarray,
    qubits: Tuple['cirq.Qid', ...],
    dtype: Type[np.complexfloating],
-) -> np.ndarray:
+) -> Optional[np.ndarray]:


Please revert, the return value on line 2627 can be only None if protocols.apply_unitaries is called with default=None; without search argument it raises TypeError. #5669

dabacon

Few minor comments. You may have conflicts with #5669

dabacon · 2022-07-07T00:48:59Z

cirq-core/cirq/linalg/combinators.py

@@ -15,14 +15,14 @@
 """Utility methods for combining matrices."""

 import functools
-from typing import Union, TYPE_CHECKING


nit: abc imports (also below)

Not needed anymore. Reverted.

dabacon · 2022-07-07T00:59:07Z

cirq-core/cirq/linalg/combinators.py

@@ -39,7 +39,9 @@ def kron(*factors: Union[np.ndarray, complex, float], shape_len: int = 2) -> np.
    Returns:
        The kronecker product of all the inputs.
    """
-    product = np.ones(shape=(1,) * shape_len)
+    product: np.ndarray[


I think this can just be np.ndarray (worked locally for me). The extra typing likely is just more confusing than helpful.

dabacon · 2022-07-07T00:59:43Z

cirq-core/cirq/circuits/circuit.py

@@ -1054,6 +1054,7 @@ def unitary(
        state = qis.eye_tensor(qid_shape, dtype=dtype)

        result = _apply_unitary_circuit(self, state, qs, dtype)
+        assert result is not None


think this is fixed in #5669

dabacon · 2022-07-07T01:02:27Z

Thanks for tackling these @vtomole . We cleared out a lot of the easy ones, the more fun ones are all that are left!

dabacon · 2022-07-07T01:15:27Z

cirq-core/cirq/qis/quantum_state_representation.py

@@ -38,7 +38,7 @@ def copy(self: TSelf, deep_copy_buffers: bool = True) -> TSelf:

    @abc.abstractmethod
    def measure(
-        self, axes: Sequence[int], seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None


slightly strange that this one is None, but. one below for sample is None. Is this coming from a subclass?

Wanted to be consistent with clifford_tableau's measure but given that it's actually not needed, I've reverted this change.

dabacon

One more comment and LGTM

dabacon · 2022-07-07T01:26:42Z

cirq-core/cirq/qis/clifford_tableau.py

@@ -583,6 +583,6 @@ def apply_global_phase(self, coefficient: linear_dict.Scalar):
        pass

    def measure(
-        self, axes: Sequence[int], seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None
+        self, axes: Sequence[int], seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = np.random
    ) -> List[int]:


I think I would keep this None and then do a seed or np.random in the parameter in self._measure. That way we don't have to change the default here (though I don't think it will matter much). np.random might be considered mutable(?) since it changes when different people use it, so best to not put mutables as defaults.

Do we want it as a parameter or like:

def _measure(self, q, prng: Optional[np.random.RandomState]) -> int: if prng is None: prng = np.random

that could also work, though this also does something I try to avoid which is re-assign a parameter. Mostly this doesn't ever hurt you, but there are cases where it does (mostly having to do with pytest, btw). It also sort of makes sense that the private method has a random state, but the public one has the default of None.

whichever way you want 👍

Ah, makes sense. Switched to param.

…ib#5668) Part of quantumlib#3767

Some numpy types

4067fba

CirqBot added the size: S 10< lines changed <50 label Jul 6, 2022

dstrain115 reviewed Jul 6, 2022

View reviewed changes

vtomole added 8 commits July 6, 2022 17:38

park

4939e91

Merge branch 'master' into numpy

db798c8

Merge branch 'numpy' of github.com:vtomole/Cirq into numpy

fdaf25b

Bring back type alias

9dec22c

Optional types

48335ae

Assert

3f6a4e0

Remove otpional

413b304

Merge branch 'master' into numpy

3aee1fd

vtomole marked this pull request as ready for review July 6, 2022 23:10

vtomole requested review from mrwojtek, a team and cduck as code owners July 6, 2022 23:10

vtomole requested review from tanujkhattar and dstrain115 July 6, 2022 23:10

vtomole added 2 commits July 6, 2022 18:17

Remove unused optional

bda1fd4

Merge branch 'numpy' of github.com:vtomole/Cirq into numpy

5d90874

pavoljuhas reviewed Jul 6, 2022

View reviewed changes

Revert

1115be5

dabacon requested changes Jul 7, 2022

View reviewed changes

vtomole added 3 commits July 6, 2022 20:03

Merge branch 'master' into numpy

4a65ac2

Review comments

d314d11

Remove assert

8aa737f

vtomole requested review from dabacon and pavoljuhas July 7, 2022 01:12

dabacon reviewed Jul 7, 2022

View reviewed changes

Revert type

d7608f4

vtomole requested a review from dabacon July 7, 2022 01:20

dabacon approved these changes Jul 7, 2022

View reviewed changes

Default param

7e831ca

vtomole added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 7, 2022

Alphabet

ae99740

CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 7, 2022

Merge branch 'master' into numpy

73c3833

CirqBot merged commit 27a7f58 into quantumlib:master Jul 7, 2022

CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 7, 2022

vtomole deleted the numpy branch July 7, 2022 02:59

rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023

Modify some numpy types to be compatible with numpy>=1.20 (quantuml…

aa9e3b1

…ib#5668) Part of quantumlib#3767

harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024

Modify some numpy types to be compatible with numpy>=1.20 (quantuml…

e794375

…ib#5668) Part of quantumlib#3767

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify some numpy types to be compatible with `numpy>=1.20` #5668

Modify some numpy types to be compatible with `numpy>=1.20` #5668

vtomole commented Jul 6, 2022

dstrain115 left a comment

dstrain115 Jul 6, 2022

vtomole Jul 6, 2022

pavoljuhas Jul 6, 2022

vtomole Jul 6, 2022

dabacon left a comment

dabacon Jul 7, 2022

vtomole Jul 7, 2022

dabacon Jul 7, 2022

vtomole Jul 7, 2022

dabacon Jul 7, 2022

vtomole Jul 7, 2022

dabacon commented Jul 7, 2022

dabacon Jul 7, 2022

vtomole Jul 7, 2022

dabacon left a comment •

edited

Loading

dabacon Jul 7, 2022

vtomole Jul 7, 2022

dabacon Jul 7, 2022

dabacon Jul 7, 2022

vtomole Jul 7, 2022

Modify some numpy types to be compatible with numpy>=1.20 #5668

Modify some numpy types to be compatible with numpy>=1.20 #5668

Conversation

vtomole commented Jul 6, 2022

dstrain115 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabacon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabacon commented Jul 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dabacon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Modify some numpy types to be compatible with `numpy>=1.20` #5668

Modify some numpy types to be compatible with `numpy>=1.20` #5668

dabacon left a comment •

edited

Loading