microsoft / microsoft/yardl

Python NDArray padding causes binary wire incompatibility

Open
#301 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
35
Forks
14
Avg merge
5d 17h
Merged PRs (30d)
1

Description

Given a Record with fields of mixed-width, e.g.

Mixed: !record
  fields:
    a: uint8
    b: float64

Python binary NDArray serializers unconditionally write arrays using numpy's aligned struct layout, which includes struct padding on the wire. But in C++, however, such a Record is considered not trivially serializable, and, thus, will be serialized item-by-item, resulting in a packed layout on the wire. The resulting streams are not compatible!

Here is the trace:

  • yardl's get_dtype() builds record dtypes with align=True, so a mixed-width record (e.g. {uint8, float64}) has inter-field padding: aligned itemsize (16) > packed field-sum (9).
  • NDArraySerializerBase._write_data takes a bulk write_bytes_directly(value.data) path when _is_current_array_trivially_serializable(value) is true, memcpy'ing the raw (padded) buffer to the wire.
  • That guard's element check bottoms out at RecordSerializer.is_trivially_serializable(), which only requires every field to be individually trivially serializable — it has no "no padding" clause.
  • So for a padded record the guard passes, and the padding bytes (uninitialized garbage, since numpy doesn't zero them) go onto the wire; _read_data mirrors this, reading _array_dtype.itemsize (aligned) bytes.
  • C++'s IsTriviallySerializable<T> gates the equivalent bulk copy on the stricter sizeof(struct) == Σ sizeof(field), so it falls back to field-by-field (packed) for the same record — the two languages disagree.
  • Result: silent cross-language break for !array of mixed-width records (C++ can't read Python's file and vice versa); Python↔Python round-trips still pass because both sides use the aligned dtype, which is why no existing test caught it.
  • Affects all three NDArray kinds (DynamicNDArraySerializer, NDArraySerializer, FixedNDArraySerializer) on both read and write, since they share NDArraySerializerBase.
  • !stream/!vector of the same record are unaffected — they never bulk-copy, always serializing field-by-field (packed).

To reproduce, we need a model that serializes arrays of records containing mixed-width fields, e.g.:

# Correct/packed reference path: streams use the element serializer directly.
ViaStream: !protocol
  sequence:
    items: !stream
      items: Mixed

# Buggy path: 1-D NDArray of the mixed-width record.
ViaArray: !protocol
  sequence:
    items: !array
      items: Mixed
      dimensions: 1
import numpy as np
import nd_array_padding_bug as m
from nd_array_padding_bug import binary as b

VALUES = [(1, 1.5), (2, 2.5), (3, 3.5)]

PY_ARRAY_FILE = "./ndarray_padding_bug_py_array.bin"
PY_STREAM_FILE = "./ndarray_padding_bug_py_stream.bin"

dtype = m.get_dtype(m.Mixed)

arr = np.array(VALUES, dtype=dtype)
with b.BinaryViaArrayWriter(PY_ARRAY_FILE) as w:
    w.write_items(arr)
with b.BinaryViaStreamWriter(PY_STREAM_FILE) as w:
    w.write_items([m.Mixed(a=a, b=x) for a, x in VALUES])
wc *.bin
  0   4 286 ndarray_padding_bug_py_array.bin
  0   2 253 ndarray_padding_bug_py_stream.bin
  0   6 539 total

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with get_dtype(), NDArraySerializerBase._write_data/_read_data, and RecordSerializer.is_trivially_serializable(), then compare them with C++ IsTriviallySerializable. Reproduce the mixed-width Mixed record using the supplied array and stream models, and verify that all three NDArray serializers use a packed, cross-language-compatible layout without breaking Python round-trips.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, numpy, python
Domain
data, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.