Fix json serialization for NDArray #11303
Conversation
| data = tvm.nd.array(np.random.rand(*shape).astype(dtype), device=dev) | ||
| body = tvm.tir.Evaluate(0) | ||
| stmt = tvm.tir.AllocateConst(buf.data, dtype, shape, data, body) | ||
| stmt2 = tvm.ir.load_json(tvm.ir.save_json(stmt)) |
There was a problem hiding this comment.
could you double-check that you're actually exporting an NDArray into JSON here? i think this might fix it , but it would be great if we could prove it through e.g. non-empty b64ndarray key
There was a problem hiding this comment.
My read is that it does not go through b64ndarray mechanism, but instead goes through repr_bytes mechanism as @vinx13 commented. The old b64ndarray mechanism should still continue to work. At some time pt perhaps we can move to repr_bytes mechanism for all cases while keeping b64ndarray for one cycle.
The structural equality in the next line should proves the serialization correctness
There was a problem hiding this comment.
I've added a check of alloc_const.data against ref data
There was a problem hiding this comment.
thanks @vinx13! did we ever figure out which mechanism is used to serialize the ndarray though?
There was a problem hiding this comment.
It will enter this path https://github.com/apache/tvm/blob/main/src/node/serialization.cc#L156 and call the registered set_repr_bytes during indexing, so it won't be saved into b64array section. The node is later saved in https://github.com/apache/tvm/blob/main/src/node/serialization.cc#L276
|
Nice, @vinx13 @junrushao1994 . Amazing. Thanks for the quick fix! |
When `NDArray` is being stored as `ObjectRef`, the serializer won't trigger the right path for storage. Under the new serialization mode, we need to be able to leverage the `repr_bytes` mechanism to save `NDArray`. This change is backward compatible -- ndarray saved in previous format will continue to work. And fixes the problem of serialization when `NDArray` is involved as part of `ObjectRef`. In the future, we can consider consolidate the `NDArray` save into the `repr_bytes` and remove the specialization as we evolve to newer versions
When `NDArray` is being stored as `ObjectRef`, the serializer won't trigger the right path for storage. Under the new serialization mode, we need to be able to leverage the `repr_bytes` mechanism to save `NDArray`. This change is backward compatible -- ndarray saved in previous format will continue to work. And fixes the problem of serialization when `NDArray` is involved as part of `ObjectRef`. In the future, we can consider consolidate the `NDArray` save into the `repr_bytes` and remove the specialization as we evolve to newer versions
When `NDArray` is being stored as `ObjectRef`, the serializer won't trigger the right path for storage. Under the new serialization mode, we need to be able to leverage the `repr_bytes` mechanism to save `NDArray`. This change is backward compatible -- ndarray saved in previous format will continue to work. And fixes the problem of serialization when `NDArray` is involved as part of `ObjectRef`. In the future, we can consider consolidate the `NDArray` save into the `repr_bytes` and remove the specialization as we evolve to newer versions
5 participants
When
NDArrayis being stored asObjectRef, the serializer won't trigger the right path for storage. Under the new serialization mode, we need to be able to leverage therepr_bytesmechanism to saveNDArray.This change is backward compatible -- ndarray saved in previous format will continue to work. And fixes the problem of serialization when
NDArrayis involved as part ofObjectRef. In the future, we can consider consolidate theNDArraysave into therepr_bytesand remove the specialization as we evolve to newer versionscc @tqchen @areusch