refactor(test): #270 shared BorrowedLayer.h builders; FLOAT32 factory gate settled as by-design#378
Merged
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]>
The executeOp OUT_WRITE epilogue now rounds by the op's arithmetic.roundingMode instead of the target's storage qConfig, implemented as a transient swap of the target's storage rounding slot scoped to the epilogue conversion (conversionMatrix signatures untouched, qConfig restored before returning). The storage qConfig roundingMode remains the default the op's rounding derives from (arithmeticFromQuantization), and stays authoritative for bare conversions (executeConvert/convertTensor), the OUT_ACC_* epilogues (accumulator-grid discipline, D4), storage/inference encodes and serialization. The optimizer migrates off the executeOpWithEpilogueRounding shim (now deleted): sgdStepM/adamWStep set arithmetic.roundingMode = optim->writeBackRounding on their write-back opSpecs and call plain executeOp. All #279 seeded-SR write-back semantics are preserved (pinned by the existing UnitTestSgd/UnitTestAdamW dead-zone tests). Back-compat invariant: every other executeOp caller derives its arithmetic from a quantization whose roundingMode equals the produced wire's storage mode, so the epilogue-rounding source change is observationally identical there (full suite green, 81/81). Closes #282 Co-Authored-By: Claude Fable 5 <[email protected]>
… gate is by-design Consolidate the eight file-local buildBorrowedLinearLayer / buildBorrowedConv1dLayer / freeLinearLayerShellOnly copies into test/unit/support/include/BorrowedLayer.h (new odt_test_layer_builders INTERFACE lib, DeathTest.h precedent) and migrate all consumers, including the hand-wired SYM-storage site in UnitTestLayerWeightsApi and the buildTrainableLinear tail in UnitTestQuantLayerIntegration. Public factory API unchanged per maintainer decision on #270: requireFloat32 stays; FEATURES.md, LayerCommon.c and the mixed_width_mlp example now present FLOAT32-init + requantizeTensorInPlace() (or LayerNorm/GroupNorm constant-fill) as the documented path to SYM_INT32-native params. Co-Authored-By: Claude Fable 5 <[email protected]>
LeoBuron
force-pushed
the
270-symint-param-test-helper
branch
from
July 20, 2026 15:45
b14a4e6 to
f22c42b
Compare
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
Implements the maintainer decision on #270 (2026-07-20): the
requireFloat32factory gate is by design — no SYM_INT32-aware factory init is added to the public API. Instead, the test-side workaround is consolidated into one shared helper, and the docs now present the public-API path as the documented pattern rather than a workaround.Shared test helper
test/unit/support/include/BorrowedLayer.h(header-only,DeathTest.hprecedent) withbuildBorrowedLinearLayer,buildBorrowedConv1dLayer,freeLinearLayerShellOnlyasstatic inline; carried by a newodt_test_layer_buildersINTERFACE lib intest/unit/support/CMakeLists.txt.buildBorrowedLinearLayer: UnitTestLinear, UnitTestSgd, UnitTestAdamW, UnitTestInferenceApi, UnitTestMultiLayerTraining, UnitTestOptimizerScaling, UnitTestTrainingLoopApi (7 copies)buildBorrowedConv1dLayer: UnitTestConv1d (1 copy)freeLinearLayerShellOnly: additionally deduped from UnitTestMnistSmoke, UnitTestCalculateGradsSequential, and UnitTestQuantLayerIntegration (freeLinearLayerShell, identical body)testLayerLoadWeightsLinearQuantizesForSymStorage(UnitTestLayerWeightsApi) and the config-wiring tail ofbuildTrainableLinear(UnitTestQuantLayerIntegration)weightGradAccMode/biasGradAccModeexplicitly (the fullest of the seven variants). The four files that previously left them calloc-zeroed never exercise a backward pass on these layers (verified by grep for backward/calculateGrads/trainingRun), so behavior is identical; fixtures and assertions are untouched.initConv1dTransposedConfigWithWeightsAndBias, so nothing to consolidate there.buildSymLinearLayerin UnitTestCalculateGradsSequential stays local: it is a genuinely different two-Q (mathQ/propLossQ-divergent) shape, now documented as the two-Q variant of the shared helper.buildLinearStub(UnitTestModelValidationApi, parameter-free NULL-weights stub) and the grad-optional partial config in UnitTestLinear — both test different wiring shapes than the helper produces.Docs / example
examples/mixed_width_mlp/train_c.c: comments no longer call FLOAT32-init +requantizeTensorInPlace()"temporary" — it is the documented public-API path to SYM_INT32-native params (structure unchanged).docs/FEATURES.md: table cells✗ (#270 gate)→~ requantize path (#270); "Quant params" note and Known-gaps bullet rewritten to the settled design.src/userApi/LayerCommon.c: rationale comment atrequireFloat32(comment only, no semantic change).Design rationale
Per the maintainer comment (2026-07-20), a public factory knob for SYM_INT32-native random init "does not help anyone": random init is defined on floats anyway (Kaiming/Xavier bounds, scale derivation), so any factory-native path would internally do float-init-then-quantize — exactly what the user can already do explicitly. Keeping the gate keeps the API honest about what the init actually computes.
Honest assessment — what remains unreachable via the public API
I looked for a concrete user-facing capability that FLOAT32-init + requantize or constant-fill cannot deliver. Verdict: no hard gap; two soft costs worth recording:
requantizeTensorInPlace()is example code, not a framework API. The pattern is ~15 lines over public primitives (getQLike,getDataLike,convertTensor,freeData,freeQuantization), documented inexamples/mixed_width_mlp/train_c.c. Each downstream user re-implements it. When to revisit: the moment a second example or a downstream consumer copies it, promote it toTensorApias a small utility — that is a tensor-level requantize helper, not a factory knob, so it would not conflict with this decision.deserializeTensoroverwrites a FLOAT32 factory skeleton in place with SYM_INT32 quantization + mantissas (equal 4-byte payload passes the serial: deserializeTensor trusts the file dtype — heap overflow / NULL-qConfig write when a checkpoint's dtype differs from the pre-built skeleton #316 size guard) — no float staging, no requantize step.Verification
ctest --preset unit_test_debug: 100% tests passed out of 81 (baseline before refactor: same).cmake --build --preset examples: all 395 targets build, every example binary compiles.cfg->bias = NULL→ UnitTestLinear, UnitTestSgd, UnitTestLayerWeightsApi, UnitTestQuantLayerIntegration all FAILgroups 1→2→ UnitTestConv1d FAILS.clang-format).Suggested closing comment for #270
Closes #270
🤖 Generated with Claude Code