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

JSON serialization panics when given numpy values #4030

Closed
mpharrigan opened this issue Apr 19, 2021 · 4 comments · Fixed by #4382
Closed

JSON serialization panics when given numpy values #4030

mpharrigan opened this issue Apr 19, 2021 · 4 comments · Fixed by #4382
Labels
area/json area/serialization kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@mpharrigan
Copy link
Collaborator

print(cirq.to_json({'value': 1}))
{
  "value": 1
}
print(cirq.to_json({'value': np.array(1)}))
Traceback (most recent call last):
  File "/usr/local/google/home/mpharrigan/cirq/env/lib/python3.8/site-packages/IPython/core/interactiveshell.py", line 3427, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-7-0e5a100f1b72>", line 1, in <module>
    print(cirq.to_json({'value': np.array(1)}))
  File "/usr/local/google/home/mpharrigan/cirq/cirq/cirq/protocols/json_serialization.py", line 515, in to_json
    if has_serializable_by_keys(obj):
  File "/usr/local/google/home/mpharrigan/cirq/cirq/cirq/protocols/json_serialization.py", line 445, in has_serializable_by_keys
    return any(has_serializable_by_keys(elem) for pair in obj.items() for elem in pair)
  File "/usr/local/google/home/mpharrigan/cirq/cirq/cirq/protocols/json_serialization.py", line 445, in <genexpr>
    return any(has_serializable_by_keys(elem) for pair in obj.items() for elem in pair)
  File "/usr/local/google/home/mpharrigan/cirq/cirq/cirq/protocols/json_serialization.py", line 447, in has_serializable_by_keys
    return any(has_serializable_by_keys(elem) for elem in obj)
TypeError: iteration over a 0-d array

This used to work. The above is a minimal example taken from real code that used to work with previous cirq versions.

@mpharrigan mpharrigan added the kind/bug-report Something doesn't seem to work. label Apr 19, 2021
@mpharrigan
Copy link
Collaborator Author

diff --git a/cirq/protocols/json_serialization.py b/cirq/protocols/json_serialization.py
index 77bc8d8e..b1dcdd67 100644
--- a/cirq/protocols/json_serialization.py
+++ b/cirq/protocols/json_serialization.py
@@ -444,6 +444,8 @@ def has_serializable_by_keys(obj: Any) -> bool:
     if isinstance(obj, Dict):
         return any(has_serializable_by_keys(elem) for pair in obj.items() for elem in pair)
     if hasattr(obj, '__iter__') and not isinstance(obj, str):
+        if isinstance(obj, np.ndarray) and obj.ndim==0:
+            return False
         return any(has_serializable_by_keys(elem) for elem in obj)
     return False

is my hack. I don't know what the "true" fix would be.

@balopat balopat added area/json area/serialization triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add labels May 3, 2021
@balopat
Copy link
Contributor

balopat commented Jul 30, 2021

I have no better idea than your fix either. If feel like this is a highly special case where numpy is kind of breaking the iterable contract by having __iter__ on a class which can be initialized to be non-iterable...
I say let's special case it to np.ndarray, ndim==0. Alternatively we could catch TypeError, but I prefer the explicit special logic!

@mpharrigan
Copy link
Collaborator Author

We could EAFP and try/catch Type error around the any()

@95-martin-orion
Copy link
Collaborator

I have mixed feelings about using EAFP (it could lead to hard-to-find performance hits, depending on implementation), but overall agree with the proposed solution. The intent of this code is to locate key-serializable objects (like FrozenCircuit) in iterables; anything that would never contain such an object can be skipped.

CirqBot pushed a commit that referenced this issue Aug 5, 2021
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/json area/serialization kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
3 participants