compass(kv): refuse numpy's boolean as a parallel width (#341) - #348
Conversation
whole_number refused a Python bool but took numpy.bool_(True) as the width 1. numpy's boolean is not a bool subclass. It is now refused, with the same named ValueError. The check is duck-typed on `dtype == bool`, so handoff.py still does not import numpy. It also refuses a 0-d numpy bool array. A type-name check would not work: on numpy 2 the type is named "bool", not "bool_". The docstring's Refused list drops "text;", which the paragraph above it already covers. The [float] case of the whole-width test is gone. It repeated test_the_ranks_the_router_reads_are_numbers, and one mutation fails both. The test is renamed to say what it now takes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| and anything else `int` does not take exactly. No value that is accepted | ||
| changes on the way to the `int` returned, and that check is what refuses | ||
| integer text: `int("8")` is 8, which is not equal to "8". | ||
| naming a deployment that was never launched; a truth value, which would |
There was a problem hiding this comment.
Blocking. The docstring says truth values are refused, but torch's boolean goes out as a width of 1 or 0. (Principle 8: "Every claim carries its measurement." Principle 6: "Refuse rather than fall back.")
The sentence reads "Refused: ... a truth value, which would go out as a width of 1 or 0 -- a bool, or numpy's boolean". I ran whole_number("tp_size", v) on node 18 (xiaobizh_n18_cpu: numpy 2.4.6, torch 2.10.0+rocm7.2.4, pandas 2.3.3), at the merged tree c0249e0d7:
| input | head | tip's handoff.py |
|---|---|---|
torch.tensor(True) |
accepted, 1 (int) |
accepted, 1 |
torch.tensor(False) |
accepted, 0 (int) |
accepted, 0 |
numpy.bool_(True), numpy.array(True) |
refused, named | accepted, 1 |
torch.bool == bool is False, so the line-102 check never fires on a torch tensor. ATOM's own _integer in atom/kv_transfer/offload/hybrid/dsv4/codec.py refuses torch.Tensor outright. That is the one column where the PR body's comparison table leaves out the codec helper.
The PR body records this under "Left alone". But AI_DEV_RULES says: "A finding not fixed in the PR that found it gets an issue: PR bodies are squashed away on landing." Once this lands, the docstring is the only record left, and it says the opposite.
Either fix is fine (about 1-2 lines):
- Widen the check so it matches the sentence:
isinstance(value, bool) or "bool" in str(getattr(value, "dtype", "")). I measured this on node 18.- It refuses
numpy.bool_(True),numpy.array(True),torch.tensor(True),torch.tensor(False),pd.Series([True])andpd.array([True], dtype="boolean"). - It does not refuse
numpy.int64(8),numpy.array(8),torch.tensor(8),Decimal("8"),8,8.0ornumpy.float64(8.0). - Add a
torch.tensor(True)refusal case. It must be red with today's line 102.
- It refuses
- Narrow the sentence to what is refused, e.g. "a Python
bool, or anything whosedtypeequals numpy'sbool", say that a torch bool tensor is not refused, and file the issue.
There was a problem hiding this comment.
Fixed in 2254c18d6: the check is widened, and the sentence now says exactly what it does.
- Line 102 is now
isinstance(value, bool) or "bool" in str(getattr(value, "dtype", "")).handoff.pystill imports neither numpy nor torch: its only imports are__future__.annotationsandtyping.Any. - The docstring now says: "a boolean, which would go out as a width of 1 or 0 -- a
bool, or anything whosedtypehas "bool" in its text, as numpy's and torch's booleans do, neither being aboolsubclass". That is the mechanism, word for word. - New case:
test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-torch-true].torchis imported at the top of the test file, as it is in five othertests/compass/files. It is present inxiaobizh_n18_cpu, so noimportorskip.
Seen firing on node 18. I ran tests/compass/test_kv_remote_prefill.py against the merged tree c940a2e30 (tip cb684287f, stamp 4370e9477), swapping only handoff.py each time, 112 lines in every variant:
| line 102 | result | failing node ids |
|---|---|---|
| new (head) | 45 passed, rc 0 | none |
83584d76b's getattr(value, "dtype", None) == bool |
1 failed, 44 passed, rc 1 | ...[tp_size-torch-true], Failed: DID NOT RAISE <class 'ValueError'> |
the tip's isinstance(value, bool) |
3 failed, 42 passed, rc 1 | ...[tp_size-numpy-true], ...[tp_size-numpy-false], ...[tp_size-torch-true] |
Direct probe of whole_number("tp_size", v) (numpy 2.4.6, torch 2.10.0):
torch.tensor(True)andtorch.tensor(False): refused and named at the head. With83584d76b's line they were accepted as1and0.- No integer dtype is refused. Every integer dtype that can hold 8 was accepted as
8(int): 10 numpy types,int8throughuint64andlonglong/ulonglong, and 8 torch dtypes,torch.int8throughtorch.uint64. That includesnumpy.int64,numpy.uint8andtorch.int64. Decimal("8"),Decimal("8.0"),8and8.0are still8.
| """ | ||
| try: | ||
| if isinstance(value, bool): | ||
| if isinstance(value, bool) or getattr(value, "dtype", None) == bool: |
There was a problem hiding this comment.
Non-blocking, and no action needed. The new comparison sits inside the try, but it can raise something the except does not catch. (Principle 6: "A declined answer with a named reason is a result.")
except (TypeError, ValueError, OverflowError) does not catch an exception raised by a foreign dtype's __eq__, or by a dtype property. On node 18, with an int subclass holding the value 8:
dtype behaviour |
head | tip |
|---|---|---|
__eq__ raises RuntimeError |
crashes with RuntimeError, and the field is not named |
accepted, 8 |
the property raises RuntimeError |
crashes with RuntimeError |
accepted, 8 |
__eq__ always returns True |
refused, named | accepted, 8 |
__eq__ returns a 2-element array |
refused, named (the ambiguous-truth ValueError is caught) |
accepted, 8 |
Every real library I measured behaves: numpy.int64(8), numpy.array(8), torch.tensor(8) and pd.array([8], dtype="Int64")[0] are all accepted as 8. pd.Series([True]) goes from an unnamed ambiguous-truth ValueError at the tip to a named refusal at the head.
So this is recorded for the next person, not asked for. The only callers, connector.py:264-265, pass Config ints.
There was a problem hiding this comment.
No change, as you advised. The widened line still reads the dtype inside the try. A foreign dtype whose __str__ or property raises something other than TypeError, ValueError or OverflowError would still escape unnamed. As you measured, no real library does this, and the only callers, in connector.py, pass Config ints.
| @pytest.mark.parametrize("value", [8, 8.0], ids=["int", "float"]) | ||
| def test_a_whole_width_is_taken_as_an_int_or_a_whole_float(geometry, value): | ||
| """The refusal above is not of an `int` or of a float with no fraction.""" | ||
| def test_a_whole_width_is_taken_as_an_int(geometry): |
There was a problem hiding this comment.
Non-blocking. ponytail delete:: this test pins nothing that nine other tests in this file do not already pin. (AI_DEV_RULES gate 4: "A check counts only once someone has seen it fire.")
The test's claim is "The refusal above is not of an int." I tested it with a mutant that refuses every int: isinstance(value, bool) becomes isinstance(value, int) on line 102 of handoff.py, with the line count kept at 112. On node 18 the file went from 45 passed to 15 failed, 30 passed:
- this test failed.
- 9 other connector-building tests failed, because they take the default int width. Among them are
test_a_finished_request_carries_the_blob_outandtest_a_remote_filled_request_parks_and_leaves_on_its_deadline. - 5
dp_rank-*refusal cases failed, because thetp_size=8in that test's ownwidthsis refused first.
I found no plausible mutant that only this test catches. Its type(...) is int assertion adds nothing for an int input: return value returns the same int.
delete: lines 316-328 plus the two blank lines. Nothing replaces them. This is not required. The brief only named [float].
There was a problem hiding this comment.
Applied in 2254c18d6. test_a_whole_width_is_taken_as_an_int is deleted, with its two trailing blank lines: 16 lines.
First I checked that nothing in the named result depended on it. On node 18, against the merged tree c940a2e30 without it:
- The
return whole->return valuemutation is still red, ontests/compass/test_kv_remote_prefill.py::test_the_ranks_the_router_reads_are_numbers(assert (False),isinstance(8.0, int)). That is 1 failed, 44 passed. - The null control,
return int(whole), is 45 passed. - Integer inputs are still accepted as
int: the probe gives8for every numpy and torch integer dtype.
|
Agent-authored review (reviewer agent, cycle 1). I read the eight design principles in Verdict: REQUEST_CHANGES on head
1. Named result, reproduced on node 18 (
|
| variant | handoff.py lines |
result | failing node ids |
|---|---|---|---|
| head as merged | 112 | 45 passed, rc 0 | none |
tip's whole file put back (git show f21492580:...) |
111 | 2 failed, 43 passed, rc 1 | ...::test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-numpy-true], ...[tp_size-numpy-false] |
only line 102 reverted to if isinstance(value, bool): |
112 (line count kept) | 2 failed, 43 passed, rc 1 | the same two |
return whole -> return value |
112 | 1 failed, 44 passed, rc 1 | ...::test_the_ranks_the_router_reads_are_numbers |
null: return whole -> return int(whole) |
112 | 45 passed, rc 0 | none |
Direct probe of whole_number("tp_size", v) (numpy 2.4.6, torch 2.10.0, pandas 2.3.3):
- Still accepted as
8(int) at the head:numpy.int64(8),Decimal("8"),Decimal("8.0"),Fraction(8, 1),numpy.float64(8.0)andnumpy.array(8). - Refused and named at the head, accepted at the tip:
numpy.bool_(True),numpy.bool_(False)andnumpy.array(True). - The refusal text is unchanged:
'tp_size is np.True_, which is not a whole number; a parallel width or rank is a count, so it is refused rather than converted'. The template lines are not in the diff. - Other body claims, measured:
type(numpy.bool_(True)).__name__is'bool'.issubclass(numpy.bool_, bool)isFalse.numpy.dtype('bool'),numpy.dtype('?')andnumpy.dtype(numpy.bool_)all== bool.0and-1are accepted, as the body says.
2. Attack on getattr(value, "dtype", None) == bool
| input / comparison | result |
|---|---|
numpy.dtype('bool') == bool |
True, no warning |
numpy.dtype('int64') / ('O') == bool |
False |
torch.bool == bool |
False, so the check misses torch (finding 1) |
pd.BooleanDtype(), pd.Int64Dtype(), pd.CategoricalDtype() == bool |
False, no crash |
torch.tensor(True) / torch.tensor(False) |
accepted, 1 / 0, at the head and the tip |
torch.tensor(8), torch.tensor(8.0) |
accepted, 8 |
torch.tensor([8, 9]) |
refused, named |
pd.Series([True]) |
refused, named (at the tip: an unnamed ambiguous-truth ValueError) |
pd.array([True], dtype="boolean")[0] (a numpy.bool_) |
refused, named (at the tip: accepted, 1) |
pd.array([8], dtype="Int64")[0] |
accepted, 8 |
pd.Series([8]) |
unnamed ambiguous-truth ValueError at the head and the tip. The comparison sits outside the try. Not introduced here. |
an int subclass holding 8 whose dtype.__eq__ raises RuntimeError |
crashes with RuntimeError at the head, accepted at the tip (finding 2) |
the same, with a dtype property that raises |
crashes with RuntimeError at the head |
the same, with dtype.__eq__ always True |
refused, named: a valid width refused. It is refused, not guessed. |
the same, with dtype.__eq__ returning a 2-element array |
refused, named (the ValueError is caught) |
Could a real valid width be wrongly refused? None I could find. numpy.int64, numpy.array(8), torch.tensor(8) and pandas Int64 are all accepted.
A widened alternative for finding 1, measured on node 18: isinstance(value, bool) or "bool" in str(getattr(value, "dtype", "")).
- It refuses the numpy, torch and pandas booleans above.
- It lets through every integral input above, both torch ints included.
3. The removed [float] case leaves no gap
return whole->return valueis red on the ranks test only: 1 failed, 44 passed (table above). That test passes8.0/3.0through the connector and assertsisinstance(..., int).- Refusing whole floats is caught by the same test.
isinstance(value, (bool, float))on line 102, 112 lines, gave 1 failed, 44 passed, ontest_the_ranks_the_router_reads_are_numbers. - What
[float]asserted that the ranks test does not:type(...) is int, notisinstance. That only matters for a mutant returning anintsubclass, and I found no plausible one.
4. Docstring truth (principle 8)
| sentence | verdict |
|---|---|
handoff.py: "a truth value, which would go out as a width of 1 or 0" |
False for torch.tensor(True) (finding 1) |
handoff.py: "numpy's boolean, which is not a bool subclass" |
true: issubclass(numpy.bool_, bool) is False |
handoff.py: "and so is known by its dtype" |
true of the mechanism: the dtype of a numpy.bool_ is == bool |
handoff.py: "that check is what refuses integer text: int("8") is 8, which is not equal to "8"" (reflowed, unchanged) |
true: "8" and numpy.str_("8") are refused |
test: "A bool, Python's or numpy's, would go out as a width of 1 or 0" |
true at the tip: numpy.bool_ gave 1/0. A plain int(True) == True would pass the equality check. |
test: "The refusal above is not of an int." |
true, and pinned (finding 3) |
the shrink: (Refused list drops text;) |
nothing is lost: "Text is refused, "8" included" stays in the first paragraph |
No design-doc references at the head over the whole file set: a grep over both files returned 0.
5. ponytail-review
tests/compass/test_kv_remote_prefill.py:L316-328: delete: test_a_whole_width_is_taken_as_an_int.The refuse-every-int mutant fails this test and 14 other tests in the file. Nothing replaces it.atom/compass/kv/handoff.py:L102: one line. Lean.atom/compass/kv/handoff.py:L93-99: the docstring is one sentence longer than before, and it carries the mechanism. Lean.
net: -15 lines possible.
6. Gate: the merged tree, once
| field | value |
|---|---|
| tip read at review time | f21492580525601f8e6e76efcd4e89b9e95eda1a |
git merge-tree --write-tree f21492580 83584d76b |
c0249e0d774e870562b66e841d0dabd685ea6443, clean. It matches the PR body. |
| stamp | git commit-tree gave c38269b7b; the gate printed commit: c38269b7b (stamp) |
.compass-changed |
atom/compass/kv/handoff.py, tests/compass/test_kv_remote_prefill.py |
gpu: line |
not required (.compass-changed stamp) |
atom.__file__ |
/tmp/pr348r1/merged/ATOM/atom/__init__.py |
| staging | a git archive, piped through docker exec -i ... tar; md5 matched on both ends; own path; the shared mount was not touched |
| script | the tree's own scripts/compass/gate_cpu.sh, under timeout -k 10 3000, unpiped |
| result | 5259 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0, 200 s wall. This equals the developer's merged count at the same tip. |
Timing classes, from junit. All passed, so no re-run was needed:
TestTheRegionIsNotCopiedPerChunk: 4 of 4.TestNoSizeAtWhichACallStopsBeingOne::...[minimax]: 2 of 2.test_gc_utils.py::test_freezing_twice_is_additive_and_harmless.
The PR's nodes, from junit. All passed:
[tp_size-numpy-true][tp_size-numpy-false]test_a_whole_width_is_taken_as_an_inttest_the_ranks_the_router_reads_are_numbers
To reach APPROVE: fix finding 1, either by widening the check (with a torch.tensor(True) refusal case that is red against today's line 102), or by narrowing the sentence and filing the torch hole as an issue. Findings 2-4 are optional.
whole_number refused a numpy boolean by `dtype == bool`, which torch's `torch.bool` does not equal, so torch.tensor(True) went out as a width of 1 while the docstring said booleans were refused. The check now refuses any value whose dtype has "bool" in its text: numpy's `bool` and torch's `torch.bool` both do, and no integer dtype does. handoff.py still imports no torch. The docstring states that mechanism exactly. The test gains a torch.tensor(True) refusal case and drops test_a_whole_width_is_taken_as_an_int, which a refuse-every-int mutant fails only alongside fourteen other tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Developer round 2: head
Lines this round (
Named result (node 18,
|
| line 102 / variant | result | failing node ids (tests/compass/test_kv_remote_prefill.py::) |
|---|---|---|
| new head | 45 passed, rc 0 | none |
83584d76b's ... or getattr(value, "dtype", None) == bool |
1 failed, 44 passed, rc 1 | test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-torch-true], Failed: DID NOT RAISE <class 'ValueError'> |
the tip's if isinstance(value, bool): |
3 failed, 42 passed, rc 1 | ...[tp_size-numpy-true], ...[tp_size-numpy-false], ...[tp_size-torch-true] |
mutation return whole -> return value |
1 failed, 44 passed, rc 1 | test_the_ranks_the_router_reads_are_numbers, assert (False), where isinstance(8.0, int) |
null return whole -> return int(whole) |
45 passed, rc 0 | none |
Direct probe of whole_number("tp_size", v), head vs 83584d76b's line:
torch.tensor(True)/torch.tensor(False): refused, named at the head; accepted as1/0before.numpy.bool_(True)/numpy.bool_(False): refused, named in both.numpy.int64(8),numpy.uint8(8),torch.tensor(8, dtype=torch.int64),Decimal("8"),Decimal("8.0"),8,8.0: accepted as8(int) in both.- The substring refuses no integer dtype. Every integer dtype that can hold 8 was accepted as
8: all 10 numpy integer types (int8...uint64,longlong,ulonglong) and 8 torch dtypes (torch.int8...torch.uint64). The count not accepted is 0. The sub-byte and quantized torch dtypes cannot build a tensor of 8, so they were not tested. handoff.pystill imports no torch. Its only imports arefrom __future__ import annotationsandfrom typing import Any.
Lint: ruff check and black --check on both files give RUFF_RC=0 and BLACK_RC=0. Design-doc references: 0 over both files at the head.
Gate 1: the merged tree, once
| field | value |
|---|---|
| tip read after the push | cb684287fa1b0af08571c0791bbe8eaa55b55820 (#349, docs only: 08, 16) |
git merge-tree --write-tree cb684287f 2254c18d6 |
c940a2e30c7b6bcbf10dacd1eb8b80d354deae65, clean |
| stamp | git commit-tree gave 4370e9477. The gate printed commit: 4370e9477 (stamp) and gpu: not required (.compass-changed stamp) |
atom.__file__ |
/tmp/i341r2/merged/ATOM/atom/__init__.py |
| script | the tree's own scripts/compass/gate_cpu.sh, under timeout -k 10 3000, unpiped, one run |
| result | 5259 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0, 195 s |
Node-id delta from junit, against round 1's merged gate at f21492580 (5259 passed). The total is unchanged:
- tests.compass.test_kv_remote_prefill::test_a_whole_width_is_taken_as_an_int+ tests.compass.test_kv_remote_prefill::test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-torch-true]- No other node id or outcome changed.
I had also gated this head merged onto f21492580 (tree 653e5b35f, stamp 16dcaa0dd) before the tip moved. Its node-id list is identical to this one.
Timing classes passed on both runs, 7 of 7 nodes, so no re-run was needed:
TestTheRegionIsNotCopiedPerChunkTestNoSizeAtWhichACallStopsBeingOne::...[minimax]test_gc_utils.py::test_freezing_twice_is_additive_and_harmless
Next: a delta review of 83584d76b..2254c18d6.
| """ | ||
| try: | ||
| if isinstance(value, bool): | ||
| if isinstance(value, bool) or "bool" in str(getattr(value, "dtype", "")): |
There was a problem hiding this comment.
Non-blocking, no action needed. I attacked the substring check, and it errs only toward refusing. (Principle 6: "Refuse rather than fall back.") Nothing I measured hands out a guessed width that round 1's line refused.
Setup: whole_number("tp_size", v) on node 18 (xiaobizh_n18_cpu: numpy 2.4.6, torch 2.10.0+rocm7.2.4, pandas 2.3.3, pyarrow 25.0.1), at the merged tree c940a2e30. Each input was compared against 83584d76b's whole handoff.py.
Which real dtypes have "bool" in str(dtype):
- numpy: only
bool, out of 22 distinct dtypes. - torch: only
torch.bool, out of 46 dtypes. - pandas:
boolean,Sparse[bool, False]andbool[pyarrow].Int64,Int8,UInt64,category,Sparse[int64, 0]andint64[pyarrow]do not.
| input | str(dtype) |
head | 83584d76b |
|---|---|---|---|
torch.tensor(True) / (False) / ([True]) |
torch.bool |
refused, named | accepted, 1 / 0 / 1 |
pd.Series([True], dtype="boolean") |
boolean |
refused, named | unnamed ambiguous-truth ValueError |
pd.array([True], dtype="boolean"), and its [0] |
boolean / bool |
refused, named | refused, named |
pd.array([8], dtype="Int64")[0] |
int64 |
accepted, 8 | accepted, 8 |
pd.array([8], dtype="Int64") (length 1) |
Int64 |
refused, named | refused, named |
pd.Series([8], dtype="Int64") |
Int64 |
unnamed ambiguous-truth ValueError |
the same. This was already there, and the check is not involved. |
numpy.array(8, dtype=object) |
object |
accepted, 8 | accepted, 8 |
numpy.array(True, dtype=object) |
object |
accepted, 1 | accepted, 1 |
structured numpy.array((8,), [("bool", "i8")]), and the same with field x |
[('bool', '<i8')] |
refused, named | refused, named. int() refuses both, so the substring decides nothing here. |
union dtype ("i8", [("bool", "i8")]), 0-d, holding 8 |
(numpy.int64, [('bool', '<i8')]) |
refused, named | accepted, 8 |
the same union dtype with the field named x |
(numpy.int64, [('x', '<i8')]) |
accepted, 8 | accepted, 8 |
int64 with metadata={"bool": 1} |
int64 |
accepted, 8 | accepted, 8 |
numpy.timedelta64(8) |
timedelta64 |
accepted, 8 | accepted, 8 |
a pyarrow bool scalar via pandas, pa.scalar(True), pa.scalar(8) |
none | refused, named | refused, named |
an int subclass holding 8, dtype.__str__ returning "not_a_boolean_but_named_so" |
that | refused, named | accepted, 8 |
an int subclass holding 8, dtype.__str__ raising RuntimeError |
raises | crashes, RuntimeError |
accepted, 8 |
What this shows:
- No real integer dtype contains "bool". The only valid width it refuses is a union dtype with a field named
bool, or a synthetic dtype name. That is a refusal with a name, not a guess, and it is exactly what the docstring says the check does. - The one boolean that still goes out as 1 is an object-dtype array holding
True. That is unchanged from round 1 and from the tip. The docstring's dash clause names the mechanism exactly, so the sentence stays true. - A
dtypewhose__str__raises escapes unnamed. That is round 1's finding 2 again, reached through__str__instead of__eq__, and already recorded in the PR body.
Cost, measured:
- a plain
8: 0.15 µs per call. numpy.int64(8): 2.7 µs;torch.tensor(8): 2.3 µs;numpy.array(8): 3.5 µs.- a 20,000-field structured dtype (
len(str(dtype))= 368,890): refused in 280 ms. - a
dtype.__str__returning 200 MB: accepted in 67 ms.
None of these reaches the only callers, connector.py, which pass Config ints.
|
Agent-authored review (reviewer agent, cycle 2). I read the eight design principles in Verdict: APPROVE on head This covers the delta
1. Named result, reproduced on node 18 (
|
| line 102 / variant | result | failing node ids (tests/compass/test_kv_remote_prefill.py::) |
|---|---|---|
| head | 45 passed, rc 0 | none |
round 1's ... or getattr(value, "dtype", None) == bool |
1 failed, 44 passed, rc 1 | test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-torch-true]: Failed: DID NOT RAISE <class 'ValueError'> |
the tip's if isinstance(value, bool): |
3 failed, 42 passed, rc 1 | ...[tp_size-numpy-true], ...[tp_size-numpy-false], ...[tp_size-torch-true], all DID NOT RAISE |
return whole -> return value |
1 failed, 44 passed, rc 1 | test_the_ranks_the_router_reads_are_numbers: assert (False) |
null: return int(whole) |
45 passed, rc 0 | none |
Integer dtypes are still accepted. All 10 numpy integer types (as scalars, and as 0-d arrays) and all 8 torch integer dtypes returned int 8. The deletion of test_a_whole_width_is_taken_as_an_int leaves the return value mutant caught, as the table shows.
2. The substring check, attacked (principle 6)
The full table is inline on handoff.py:102. The key rows:
| question | measured |
|---|---|
| Does any integer-like dtype contain "bool"? | No. numpy: only bool of 22 dtypes. torch: only torch.bool of 46. pandas: boolean, Sparse[bool, False] and bool[pyarrow]; not Int64, UInt64 or category. |
pandas nullable Int64 |
pd.array([8], "Int64")[0] is accepted as 8. A length-1 Int64 array is refused, named. A pd.Series([8], dtype="Int64") gives an unnamed ambiguous-truth ValueError, the same with round 1's line. That comes from the comparison outside the try. |
| object dtype | numpy.array(8, dtype=object) is accepted as 8. numpy.array(True, dtype=object) is accepted as 1, the same with round 1's line. |
structured dtype with a field named bool |
refused, named, but int() refuses it anyway: the same record with field x is refused too. The only valid width newly refused is a union dtype ("i8", [("bool", "i8")]) holding 8. It is refused with a name, not guessed. |
str(dtype) raises |
crashes with RuntimeError, unnamed (synthetic type) |
str(dtype) huge |
a 369 KB dtype string: refused in 280 ms. A 200 MB __str__: accepted in 67 ms. A plain 8 costs 0.15 µs per call, and a numpy or torch scalar 2.3-3.5 µs. |
pandas "boolean" |
refused, named (pd.Series([True], dtype="boolean"), and a length-1 array). It should be refused, because it is a boolean. With round 1's line, the Series gave an unnamed ambiguous-truth ValueError. |
3. The top-level import torch in the test file
It is safe, and it changes nothing about which tests collect. importorskip would be less honest here, because it can never fire.
variant of test_kv_remote_prefill.py |
torch importable | torch hidden (sys.modules["torch"] = None) |
|---|---|---|
head (import torch) |
45 collected, rc 0 | rc 4: ImportError while loading conftest 'tests/conftest.py' |
import torch and the torch case removed |
44 collected, rc 0 | rc 4, the same error |
torch = pytest.importorskip("torch") |
45 collected, rc 0 | rc 4, the same error |
Why, measured:
tests/conftest.pyimportsatom.model_engine.scheduler, and torch is insys.modulesafter importing either one.- So without torch, nothing under
tests/collects at all. The file's own import cannot be the reason a module fails or skips, and animportorskipwould never be reached. tests/compasscollects 1309 tests at the head, against 1308 without the import and the case. The difference is the one new case.handoff.pyitself still does not load torch: a fresh interpreter importing it has'torch' in sys.modules == False.- The claim that five other
tests/compass/files import torch at top level is true:test_capture_real_model,test_kv_budget_engine,test_memory_compare,test_memory_readingsandtest_runner_non_allocating.
4. Docstring truth (principle 8)
| sentence | measured | verdict |
|---|---|---|
handoff.py: "a boolean, which would go out as a width of 1 or 0" |
With no bool check (int(v), then !=), True, numpy.bool_(True) and torch.tensor(True) give 1, and the Falses give 0. |
true |
"-- a bool, or anything whose dtype has "bool" in its text" |
This is the check, word for word. A synthetic dtype whose text is not_a_boolean_but_named_so is refused, as the sentence says. |
true |
| "as numpy's and torch's booleans do" | str(numpy.dtype(bool)) is 'bool', and str(torch.bool) is 'torch.bool'. |
true |
"neither being a bool subclass" |
issubclass(numpy.bool_, bool) and issubclass(torch.Tensor, bool) are both False. |
true |
the reflowed tail, "No value that is accepted changes ... int("8") is 8, which is not equal to "8"" (unchanged) |
"8" and " 8 " are refused. |
true |
| test: "A boolean, Python's, numpy's or torch's, would go out as a width of 1 or 0." | the no-check row above | true |
No design-doc references at the head over the PR's whole file set: the grep gives 0 on both files.
5. ponytail-review over 83584d76b..2254c18d6
atom/compass/kv/handoff.py:L102: one line, and both disjuncts are needed, since a Pythonboolhas nodtype. Nothing to cut.atom/compass/kv/handoff.py:L93-99: the same seven lines as before, reflowed. Nothing to cut.tests/compass/test_kv_remote_prefill.py:L32,L295: one import and one case. Each is the minimum for the new refusal, and the case is seen firing.tests/compass/test_kv_remote_prefill.py: the deletedtest_a_whole_width_is_taken_as_an_intwas cycle 1'sdelete:, and it is applied.
Lean already. Ship.
6. Gate: the merged tree, once
| field | value |
|---|---|
| tip read at review time | cb684287fa1b0af08571c0791bbe8eaa55b55820, unchanged since round 2 |
git merge-tree --write-tree cb684287f 2254c18d6 |
c940a2e30c7b6bcbf10dacd1eb8b80d354deae65, clean. It matches the developer's. |
| stamp | git commit-tree gave d7d90761d. The gate printed commit: d7d90761d (stamp) |
.compass-changed |
atom/compass/kv/handoff.py, tests/compass/test_kv_remote_prefill.py |
gpu: line |
not required (.compass-changed stamp) |
atom.__file__ |
/tmp/pr348r2/merged/ATOM/atom/__init__.py |
| staging | a git archive, piped through docker exec -i ... tar, into my own path. The shared mount was not touched. |
| script | the tree's own scripts/compass/gate_cpu.sh, under timeout -k 10 3000, unpiped, one run |
| result | 5259 passed, 155 skipped, 3 xfailed; GATE_CPU_RC=0, 191 s wall. It equals the developer's round-2 count. junit: 5417 tests, 0 failures, 0 errors. |
Timing classes, from junit. All passed, so no re-run was needed:
TestTheRegionIsNotCopiedPerChunk: 4 of 4.TestNoSizeAtWhichACallStopsBeingOne::...[minimax]: 2 of 2.test_gc_utils.py::test_freezing_twice_is_additive_and_harmless.
The PR's nodes, from junit. All passed:
[tp_size-numpy-true],[tp_size-numpy-false]and[tp_size-torch-true].test_the_ranks_the_router_reads_are_numbers.test_a_whole_width_is_taken_as_an_intis absent, as intended.
Landing: the approval covers tree c940a2e30 at tip cb684287f. If the tip moves, recompute git merge-tree --write-tree, as the landing rule requires.
Closes #341.
What changed
1.
whole_numbernow refuses numpy's and torch's booleans.numpy.bool_(True)/numpy.bool_(False),numpy.array(True)andtorch.tensor(True)/torch.tensor(False)all went out as the width 1 or 0.ValueError. The refusal text is unchanged.isinstance(value, bool) or "bool" in str(getattr(value, "dtype", "")). Round 1 usedgetattr(value, "dtype", None) == bool, which missed torch. The review found this, and round 2 widened it.bool, or anything whosedtypehas "bool" in its text, as numpy's and torch's booleans do, neither being aboolsubclass".2.
shrink:The docstring's Refused list dropstext;. The paragraph above already says "Text is refused, "8" included".3.
delete:Two tests are gone:[float]case of the whole-width test, in round 1. It repeatedtest_the_ranks_the_router_reads_are_numbers.test_a_whole_width_is_taken_as_an_int, in round 2, as the reviewer's ponytail suggested. A refuse-every-int mutant fails 14 other tests in the file, and thereturn valuemutant is caught by the ranks test.Lines (
git diff --numstat d96da1086 2254c18d6, from the merge base):atom/compass/kv/handoff.pytests/compass/test_kv_remote_prefill.pyMechanism, and why
handoff.pyimports neither numpy nor torch. The candidates, measured on node 18 (numpy 2.4.6, torch 2.10.0):numpy.bool_numpy.array(True)(0-d)torch.tensor(True)type(value).__name__ == "bool_"bool_integerinatom/kv_transfer/offload/hybrid/dsv4/codec.py:92-97:isinstance(value, (bool, torch.Tensor))or a numpy module + name in{"bool", "bool_"}torch.Tensor,torch.tensor(8)includedtorch(line 30)getattr(value, "dtype", None) == bool(round 1)torch.bool == boolisFalse"bool" in str(getattr(value, "dtype", ""))(chosen, round 2)8: 10 numpy types and 8 torch dtypes, with 0 refused.str(dtype)isboolfor numpy's boolean andtorch.boolfor torch's.Named result (node 18,
xiaobizh_n18_cpu)Setup:
c940a2e30(tipcb684287f+ head2254c18d6, stamp4370e9477) was staged withgit archiveanddocker exec -i ... tar -x. Per-file md5 digests matched on both ends.handoff.pywas swapped per variant, 112 lines each.atom.__file__was asserted under each root.pytest tests/compass/test_kv_remote_prefill.py.tests/compass/test_kv_remote_prefill.py::)... == bool)test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-torch-true]if isinstance(value, bool):...[tp_size-numpy-true],...[tp_size-numpy-false],...[tp_size-torch-true]return whole->return valuetest_the_ranks_the_router_reads_are_numbersreturn int(whole)Every refusal failure is
Failed: DID NOT RAISE <class 'ValueError'>.Still accepted as
8(int):numpy.int64(8),numpy.uint8(8),torch.tensor(8, dtype=torch.int64),Decimal("8"),Decimal("8.0"),8,8.0,numpy.float64(8.0),Fraction(8, 1)andnumpy.array(8). The last three were measured in round 1.Gate 1: CPU suite
Setup: the tree's own
scripts/compass/gate_cpu.sh, with.compass-commitand.compass-changedstamps, undertimeout -k 10 3000, unpiped, one run at a time. Every run printedgpu: not required (.compass-changed stamp).GATE_CPU_RCf21492580(control)f21492580c0249e0d7(ef626cf15)cb684287fc940a2e30(4370e9477)Node-id delta against the tip, from junit: round 2's merged gate against round 1's control at
f21492580. The move tocb684287fchanged only two design docs. There are no other node or outcome changes:- ...::test_a_whole_width_is_taken_as_an_int_or_a_whole_float[float]- ...::test_a_whole_width_is_taken_as_an_int_or_a_whole_float[int]+ ...::test_a_width_that_is_not_a_whole_number_is_refused_by_name[tp_size-numpy-true]+ ...[tp_size-numpy-false]+ ...[tp_size-torch-true]The timing classes passed on every run:
TestTheRegionIsNotCopiedPerChunk,TestNoSizeAtWhichACallStopsBeingOne::...[minimax]andtest_freezing_twice_is_additive_and_harmless.Lint:
RUFF_RC=0andBLACK_RC=0on both files. Design-doc references: 0 over the whole file set.Left alone
0and-1are still accepted, as compass(kv): widths written as integer text are still accepted, and a test docstring says they arrive that way #258 records. That is out of this brief.dtypethat raises in__str__or in its property escapes as that exception, not as a named refusal. This was review finding 2, and no action was taken, on the reviewer's advice. It shows up only with synthetic types. No real library does it. The only callers, inconnector.py, passConfigints.🤖 Generated with Claude Code