fix(serial): #370 ODTS wire format v2 — fixed-width LE fields, checked I/O#379
Merged
Conversation
Pin the ODTS model format to fixed-width little-endian: all counts, dims, kernel geometry, and normalizedShape fields are u32 LE (was raw host size_t), qConfig roundingMode is u8 (was host enum width), floats travel as IEEE bit patterns in u32 LE. ASYM zeroPoint is pinned as i32 LE on the wire now, so the in-memory int16->int32 widening (#246) needs no second format break; until then deserialize range-guards the narrowing. New checked SerialWire primitives (shared serial lib) fail fast on short fwrite/fread, u32 overflow on serialize, and back the same v2 encoding in the ODTR PPCA checkpoint (peek mirror updated, ODTR version also bumped to 2 since its embedded tensor records changed shape). Deserialize additionally guards: file rank vs skeleton rank (OOB dim write), LayerNorm numNormDims mismatch (OOB normalizedShape write), file dtype vs skeleton dtype (pre-existing #316 guard, now pinned by tests in both directions). SERIALIZE_FORMAT_VERSION = 2; no v1 back-compat shim (v1 files were host-local artifacts). Tests: golden-bytes fixtures pin the exact v2 encoding (tensor record, model header + RELU with SYM_INT32/ASYM qConfigs, MAXPOOL1D kernel geometry); death tests for truncated header/payload, unwritable stream, dim > u32, rank/numNormDims/dtype mismatches, oversize zeroPoint; ASYM zeroPoint round-trip. All mutation-checked (size_t re-widen, unchecked fread, i16 zeroPoint each turn a test red). Co-Authored-By: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The ODTS model format wrote every count/dimension/kernel field as raw host
size_t(8 bytes on the host, 4 on Cortex-M) with host byte order, ignored everyfwrite/freadreturn value, andqConfig.roundingModeeven leaked the host's enum width onto the wire. A model serialized on a 64-bit host was structurally unreadable on a 32-bit MCU, and a truncated file deserialized as silent garbage.This PR pins wire format v2 in one version bump:
orderOfDimensions, kernel geometry,normalizedShape, groups/outputPadding/outputSize, numNormDims/numGroups/numChannels: u32 little-endian.scale,eps, dropoutp): IEEE bit pattern as u32 LE.roundingModein qConfigs: u8 (wassizeof(roundingMode_t)= host enum width).SERIALIZE_FORMAT_VERSION = 2, existing fail-fast on version mismatch kept. No v1 back-compat shim — v1 files were host-local artifacts.Design decisions
SerialWireprimitives (new small lib insrc/serial/) —serialWrite/ReadU8|U32LE|I32LE|F32LE|Bytes+serialWriteSizeAsU32LE. Encoding is byte-by-byte LE (never a memcpy of a host integer), everyfwrite/freadis length-checked and fails fast with the module's existing idiom (PRINT_ERROR+exit(1)), andserialWriteSizeAsU32LEfail-fasts on values >UINT32_MAX. Shared by Serialize.c, Deserialize.c, andPpcaReplaySerialize.c, so the two formats cannot drift apart. Bulk tensor DATA payloads stay raw packed storage bytes; a compile-time pin inSerialWire.hrejects big-endian hosts.zeroPoint= i32 LE on the wire NOW, even though the in-memory field is stillint16_t: the parallel tensor: ASYM zeroPoint can overflow int16 for qBits>=16 with min<0 in quantizeFloatToAsym #246 PR widens the in-memory field toint32_t(lossless int16→int32), so pinning i32 here means the format breaks only once. tensor: ASYM zeroPoint can overflow int16 for qBits>=16 with min<0 in quantizeFloatToAsym #246 should merge after this PR. Until it lands, deserialize range-guards the int16 narrowing (guard drops with tensor: ASYM zeroPoint can overflow int16 for qBits>=16 with min<0 in quantizeFloatToAsym #246).PpcaReplaySerialize.cmirrors the ODTS tensor-record layout in its peek-validate-rewind path, so it had to follow; its own scalars are now LE-pinned via SerialWire andPPCA_SERIALIZE_FORMAT_VERSIONis bumped to 2 as well (its embedded tensor records changed shape — a silent-misparse hazard otherwise).numNormDimsmismatch (same OOB pattern fornormalizedShape). Dtype validation (file record vs skeletonquantization->type) existed via the serial: deserializeTensor trusts the file dtype — heap overflow / NULL-qConfig write when a checkpoint's dtype differs from the pre-built skeleton #316 guard and is now pinned by tests in both directions.Tests (TDD: each written first and observed red)
zeroPoint -3→FD FF FF FF), MAXPOOL1D kernel geometry.ASSERT_EXITS_WITH_FAILURE): truncated header, truncated DATA payload, unwritable stream on serialize.UINT32_MAXon serialize, out-of-int16-rangezeroPointon deserialize; plus an ASYMzeroPointround-trip through the i32 wire.size_t, removing thefreadlength check, and re-narrowingzeroPointto i16 each turn at least one test red; implementation restored and re-verified after each.Verification
Docs updated:
docs/FEATURES.md(serialization section + gaps list),docs/CONTINUAL_LEARNING.md(ODTR v2, portability note).Closes #370
🤖 Generated with Claude Code