-
Notifications
You must be signed in to change notification settings - Fork 19.6k
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
Numpy ndarray should be serialized as Python list. #10727
Conversation
It seems the failure case is non-relevant, let me re-trigger it. |
This assumes that the deserialization process will know about the expected array dtype. Please add a unit test. |
tests/test_model_saving.py
Outdated
def test_saving_constant_initializer_with_numpy(): | ||
""" | ||
Test saving and loading model of constant initializer with numpy ndarray as input. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format the docstring like other docstrings, e.g.
"""Test saving and loading model that includes an initializer with numpy input.
"""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you.
I believe tf.keras
would have the same issue as well. Please consider making the same PR in the TensorFlow repository.
@fchollet Sure, will do it soon. Thanks. |
Summary
If there is numpy ndarray in a layer config, for example, set
Constant(np.zeros(2,3))
as the initializer, the internal representation should be Tensor in backend(not discriminate numpy ndarray or Python list). When serializing it to JSON format, it should be regards as normal Python list. Otherwise, it will throw exception when deserialization(Refer #10718), sincefrom_config
doesn't know(doesn't need) how to deserialize numpy ndarray if there are extra message header.Note: I have verify this fix works well for #10718.
Related Issues
#10718
PR Overview