Repository navigation
Conversation
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
force-pushed
the
fix/float16-scalar-decode
branch
from
September 17, 2026 04:09
84dd170 to
05b8fd9
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.
loadbreturns an integer for a lone float16: the bytes00 4d, meaning20.0, decode as19712. A float16 inside a typed array is already correct, so this only ever affected a scalar.Both implementations have it, in the same way — the marker letter is used as the
structformat letter, andhinstructis a signed short while half ise. The C extension falls through into theTYPE_UINT16case in_bjdata_decode_value, and the pure-Python decoder mapsTYPE_FLOAT16to"h"in__UNPACK_FLOAT16and__DTYPE_MAP. The numpy dtype maps on both sides already sayf2, which is why the typed-array path escaped.Binary_JData_Specification.mdin this repo giveshas 3 bytes of IEEE 754 half in little-endian order, so the decoder contradicted the bundled spec.The fix adds
_pyfuncs_ubj_PyFloat_Unpack2beside the existingUnpack4andUnpack8. 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 ownPyFloat_Unpack2, which also returns inf/NaN.test_nd_arrayasserted the bit pattern rather than the value —bjdloadb(bjddumpb(np.float16(2.2))) == 16486— while thefloat32assertion on the following line compares againstnp.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,±infand NaN all round-trip againststruct.unpack('<e').Found by a BJData writer that emits
hfor reals that are exactly representable as a half, which is legal per the spec but something no encoder here produces —dumpbwritesdorD— so nothing exercised the scalar read path.