Skip to content

Commit 73602df

Browse files
authored
Fix gather on sliced nullable string and list tensors (#1289)
> # Summary > > Fix wrong values when gathering or materializing nullable `StringTensor`/`VarLenTensor` built from a sliced Arrow array or cuDF series. > > ```python > import pyarrow as pa, torch > from sdm import StringTensor > > s = StringTensor.from_arrow(pa.array(["LONGPREFIX", "a", None, "b"]).slice(1)) > s.tolist() # ['a', None, 'b'] > s[torch.tensor([0, 2])].tolist() # main: ['LONGPREFIX', '']; expected: ['a', 'b'] > ``` > > # Description of the change > > Root cause: for nullable inputs, `StringTensor.from_arrow`, `StringTensor.from_cudf` and `VarLenTensor.from_arrow` slice the offsets, so `_offset` is a view with its own nonzero storage offset. `_materialize`, `_layout_view` and `_valid` pass only the wrapper's storage offset to `torch.as_strided`, which addresses the underlying storage absolutely, so they read offsets (and validity) from before the slice. > > Fix: add the inner tensor's storage offset in those `as_strided` calls and subtract it again in `_from_layout_view`. This keeps inner buffers zero-copy and adds no branches, so it stays compile-friendly. Normalizing inner tensors in the constructor (cloning views) was the alternative; it copies buffers and adds a data-dependent branch for symbolic storage offsets. > > # Validation > > - New regression tests for sliced nullable Arrow input (`string`, `large_string`, list) on CPU and sliced nullable cuDF input on CUDA; the CPU tests fail on `main`. > - CPU: `test/tensor` (166 passed), `test/processing test/relational test/nn` (580 passed). > - GPU (torch 2.14.1+cu130, cuDF): `test/tensor` (219 passed), `test/processing test/relational test/nn` (826 passed), cuDF repro above returns `['a', 'b']`.
1 parent b780fc9 commit 73602df

3 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎sdm/tensor/var_len.py‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,11 +1009,13 @@ def _valid(inp: VarLenTensor) -> Tensor | None:
10091009
if inp._valid is None:
10101010
return None
10111011

1012+
# `as_strided` offsets are absolute, but `_valid` may be a view:
1013+
storage_offset = inp._valid.storage_offset() + int(inp.storage_offset())
10121014
return torch.as_strided(
10131015
inp._valid,
10141016
size=inp.size(),
10151017
stride=inp.stride(),
1016-
storage_offset=int(inp.storage_offset()),
1018+
storage_offset=storage_offset,
10171019
)
10181020

10191021

@@ -1440,22 +1442,25 @@ def _span_len(size: Sequence[int], stride: Sequence[int]) -> int:
14401442

14411443

14421444
def _layout_view(inp: VarLenTensor) -> Tensor:
1445+
# `as_strided` offsets are absolute, but `_offset` may be a view:
1446+
storage_offset = inp._offset.storage_offset() + int(inp.storage_offset())
14431447
return torch.as_strided(
14441448
inp._offset,
14451449
size=inp.size(),
14461450
stride=inp.stride(),
1447-
storage_offset=int(inp.storage_offset()),
1451+
storage_offset=storage_offset,
14481452
)
14491453

14501454

14511455
def _from_layout_view(inp: VarLenTensor, view: Tensor) -> VarLenTensor:
1456+
storage_offset = view.storage_offset() - inp._offset.storage_offset()
14521457
return inp.__class__(
14531458
data=inp._data,
14541459
offset=inp._offset,
14551460
valid=inp._valid,
14561461
size=view.size(),
14571462
stride=view.stride(),
1458-
storage_offset=0 if view.numel() == 0 else int(view.storage_offset()),
1463+
storage_offset=0 if view.numel() == 0 else int(storage_offset),
14591464
)
14601465

14611466

@@ -1494,12 +1499,7 @@ def _materialize(
14941499
non_blocking: bool = False,
14951500
) -> VarLenTensor:
14961501
# Use PyTorch's own memory-format semantics to materialize data:
1497-
start = torch.as_strided(
1498-
inp._offset,
1499-
size=inp.size(),
1500-
stride=inp.stride(),
1501-
storage_offset=int(inp.storage_offset()),
1502-
)
1502+
start = _layout_view(inp)
15031503
start = function(start)
15041504
assert start.storage_offset() == 0
15051505
assert _span_len(start.size(), start.stride()) == start.numel()
@@ -1508,7 +1508,9 @@ def _materialize(
15081508
inp._offset,
15091509
size=inp.size(),
15101510
stride=inp.stride(),
1511-
storage_offset=int(inp.storage_offset()) + 1,
1511+
storage_offset=(
1512+
inp._offset.storage_offset() + int(inp.storage_offset()) + 1
1513+
),
15121514
)
15131515
end = function(end)
15141516
assert end.storage_offset() == 0
@@ -1529,14 +1531,8 @@ def _materialize(
15291531
stride=(1,),
15301532
storage_offset=end.storage_offset(),
15311533
)
1532-
valid: Tensor | None = None
1533-
if inp._valid is not None:
1534-
valid = torch.as_strided(
1535-
inp._valid,
1536-
size=inp.size(),
1537-
stride=inp.stride(),
1538-
storage_offset=int(inp.storage_offset()),
1539-
)
1534+
valid = _valid(inp)
1535+
if valid is not None:
15401536
valid = function(valid).contiguous().view(-1)
15411537

15421538
offset, index = _compact(start, end)

‎test/tensor/test_string.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,19 @@ def test_from_arrow_zero_string_chunks(arrow_type: pa.DataType) -> None:
113113
assert tensor.to_arrow().to_pylist() == []
114114

115115

116+
@pytest.mark.parametrize("arrow_type", [pa.string(), pa.large_string()])
117+
def test_from_arrow_sliced_nullable(arrow_type: pa.DataType) -> None:
118+
tensor = StringTensor.from_arrow(
119+
pa.array(["x", "hi", None, "yo", "z"], type=arrow_type)[1:],
120+
)
121+
122+
assert tensor.tolist() == ["hi", None, "yo", "z"]
123+
assert tensor[torch.tensor([0, 2])].tolist() == ["hi", "yo"]
124+
transposed = tensor.view(2, 2).t()
125+
assert not transposed.is_contiguous()
126+
assert transposed.contiguous().tolist() == [["hi", "yo"], [None, "z"]]
127+
128+
116129
@onlyCUDA
117130
def test_from_cudf() -> None:
118131
cudf = pytest.importorskip("cudf")
@@ -148,6 +161,17 @@ def test_from_cudf_sliced_values() -> None:
148161
)
149162

150163

164+
@onlyCUDA
165+
def test_from_cudf_sliced_nullable() -> None:
166+
cudf = pytest.importorskip("cudf")
167+
168+
tensor = StringTensor.from_cudf(cudf.Series(["x", "hi", None, "yo"])[1:])
169+
170+
assert tensor.tolist() == ["hi", None, "yo"]
171+
index = torch.tensor([0, 2], device=tensor.device)
172+
assert tensor[index].tolist() == ["hi", "yo"]
173+
174+
151175
@onlyCUDA
152176
def test_from_cudf_all_empty_values() -> None:
153177
cudf = pytest.importorskip("cudf")

‎test/tensor/test_var_len.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,31 @@ def test_arrow() -> None:
216216
assert tensor.tolist() == [[1, 2], None, [3]]
217217

218218

219+
def test_arrow_sliced_nullable() -> None:
220+
tensor = VarLenTensor.from_arrow(
221+
pa.array([[0, 0, 0], [1], None, [2, 3]])[1:],
222+
)
223+
224+
assert tensor.tolist() == [[1], None, [2, 3]]
225+
assert tensor[torch.tensor([0, 2])].tolist() == [[1], [2, 3]]
226+
227+
228+
def test_sliced_offset_and_valid_views() -> None:
229+
# Rows [[0, 0, 0], [1], None, [2, 3]]; inner tensors are views from row 1.
230+
offset = torch.tensor([0, 3, 4, 4, 6])
231+
valid = torch.tensor([True, True, False, True])
232+
tensor = VarLenTensor(
233+
data=torch.tensor([0, 0, 0, 1, 2, 3]),
234+
offset=offset[1:],
235+
valid=valid[1:],
236+
size=(2,),
237+
storage_offset=1,
238+
)
239+
240+
assert tensor.tolist() == [None, [2, 3]]
241+
assert tensor[torch.tensor([1, 0])].tolist() == [[2, 3], None]
242+
243+
219244
def test_arrow_bool() -> None:
220245
tensor = VarLenTensor.from_arrow(
221246
pa.array(

0 commit comments

Comments
 (0)