Skip to content

Decode a scalar float16 as a half rather than as an int16 - #8

Open
nebkat wants to merge 1 commit into
NeuroJSON:masterfrom
nebkat:fix/float16-scalar-decode
Open

nebkat wants to merge 1 commit into
NeuroJSON:masterfrom
nebkat:fix/float16-scalar-decode

Conversation

@nebkat

@nebkat nebkat commented Sep 16, 2026

Copy link
Copy Markdown

loadb returns an integer for a lone float16: the bytes 00 4d, meaning 20.0, decode as 19712. A float16 inside a typed array is already correct, so this only ever affected a scalar.

>>> import bjdata, struct
>>> bjdata.loadb(b'{U\x01x' + b'h' + struct.pack('<e', 20.0) + b'}')
{'x': 19712}          # expected {'x': 20.0}

Both implementations have it, in the same way — the marker letter is used as the struct format letter, and h in struct is a signed short while half is e. The C extension falls through into the TYPE_UINT16 case in _bjdata_decode_value, and the pure-Python decoder maps TYPE_FLOAT16 to "h" in __UNPACK_FLOAT16 and __DTYPE_MAP. The numpy dtype maps on both sides already say f2, which is why the typed-array path escaped.

Binary_JData_Specification.md in this repo gives h as 3 bytes of IEEE 754 half in little-endian order, so the decoder contradicted the bundled spec.

The fix adds _pyfuncs_ubj_PyFloat_Unpack2 beside the existing Unpack4 and Unpack8. Unlike those two it has no native-format branch: they decode by hand only where the platform's float layout is not IEEE, and there is no corresponding C type whose layout could be in question. For the same reason a special value is returned rather than raised — the error the wider types report only arises on a platform that cannot represent it, which does not apply here. This matches CPython's own PyFloat_Unpack2, which also returns inf/NaN.

test_nd_array asserted the bit pattern rather than the value — bjdloadb(bjddumpb(np.float16(2.2))) == 16486 — while the float32 assertion on the following line compares against np.float32(2.2). It now compares against the value too.

Verified with the C extension enabled and disabled: the full suite passes (504 tests), and 0.0, -0.0, ±1.0, 20.0, 25.0, ±65504.0, the smallest normal and subnormal, ±inf and NaN all round-trip against struct.unpack('<e').

Found by a BJData writer that emits h for reals that are exactly representable as a half, which is legal per the spec but something no encoder here produces — dumpb writes d or D — so nothing exercised the scalar read path.

The scalar path read 'h' with the struct format of the same letter, which is a
signed short, so a half-precision value came back as its bit pattern: the bytes
00 4d, meaning 20.0, decoded as 19712. The typed-array path was already correct
(PyArray_HALF / dtype f2), so this only ever affected a lone value.

The specification bundled with this package gives 'h' as 3 bytes of IEEE 754
half in little-endian order, so the decoder contradicted it. The C extension
falls through to the uint16 decoder and the pure-Python fallback maps the
marker to struct 'h'; both now go through a half unpack, added beside the
existing Unpack4 and Unpack8.

The test that covered this asserted the bit pattern -- 16486 for float16(2.2) --
while the float32 assertion beside it compares against the value. It now
compares against the value too.
@nebkat
nebkat force-pushed the fix/float16-scalar-decode branch from 84dd170 to 05b8fd9 Compare September 17, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant