Improve test coverage for torch integration#928
Open
jhelferty-nv wants to merge 45 commits into
Open
Conversation
The generated Slang struct for DiffPairMarshall had two bugs preventing scalar backward passes from working: 1. Used bare `load`/`store` method names instead of `__slangpy_load`/ `__slangpy_store` (the convention used by all SlangPy Slang types). 2. Generated `__slangpy_load` that output `DifferentialPair<T>` instead of `T`, causing a type mismatch with the trampoline temporaries. The methods were also not `[Differentiable]`, so `bwd_diff` could not propagate gradients through them. The fix rewrites `generate_differential_pair` to follow the same pattern as `DiffTensor`: the struct's load/store methods operate on the primal type and, when gradients are needed, use `[Differentiable]` methods with `[BackwardDerivative]` custom backward functions that explicitly read/write the derivative buffer. Removes the "Awaiting auto-diff changes" skip markers from test_call_with_diff_scalars and test_call_with_diff_pairs, which now pass on both Vulkan and CUDA. Made-with: Cursor
- test_ndbuffer.py: 21 tests covering NDBuffer construction (empty, zeros, from_numpy, element_count, empty_like, zeros_like), data transfer (copy_from_numpy, to_numpy, clear), views, broadcasting, validation errors, and end-to-end Slang function dispatch with NDBufferMarshall (1D/2D add, scalar broadcast, read-only pass, StructuredBuffer pass, return type, int dtype). - test_diffpair_type.py: 19 tests covering DiffPair data class construction (defaults, None handling, needs_grad, int types), get/set by PrimType, slangpy_signature, and both factory functions (diffPair, floatDiffPair). Made-with: Cursor
shader-slang#782) Tests for bridge_fallback.py error/guard paths, torchtensormarshall.py DiffPair factory and error paths, builtin/tensor.py grad validation and properties, and builtin/value.py return types and reduce_type via torch pipelines. Made-with: Cursor
shader-slang#782) - Fix non-contiguous array bug in load_buffer_data_from_image when flip_y=True (np.flipud returns a view; add contiguity guard) - Add test_load_image.py: covers Tensor.load_from_image channel variants, flip_y, scale/offset, greyscale, and linearize flags - Add test_torch_dtypes.py: covers create_output branches for int32, int64, float64 in slangpytorchtensor.cpp - Add test_descriptor_marshall.py: covers DescriptorHandle low-level API and DescriptorMarshall error paths - Extend test_tensor.py: covers __str__, detach(), deprecated Tensor.numpy(), deprecated element_count param, and empty() error paths - Extend test_tensorview.py: covers slangpy.Tensor through TensorView and scalar-to-vector TensorView<float2> resolution (tensorcommon.py) - Extend test_difftensorview.py: covers slangpy.Tensor through DiffTensorView forward path (tensorcommon.py) Made-with: Cursor
…edbuffer (shader-slang#782) - test_raw_dispatch: error paths (return value, out param, no params, wrong param name), writer tuple via .write(), dispatch cache hit - test_return_types: inout scalar/vector/matrix ValueRef exercising padding/unpadding paths in valueref.py - test_modules: function.py error paths (set, write, return_type, as_struct) - test_buffers: ByteAddressBuffer read-only and RWByteAddressBuffer write Made-with: Cursor
… scalar types (shader-slang#782) Made-with: Cursor
…er-slang#782) Move tests from test_cpp_coverage_tier1.py and test_cpp_coverage_torch.py into existing files by topic: repr tests to test_tostring, texture shape to test_textures, Shape/SignatureBuilder to test_shape, buffer view indexing to test_buffer_views, torch dtypes to test_torch_dtypes, raw dispatch to test_raw_dispatch, DiffPair to test_torch_marshall_gaps, and benchmarks to a new standalone test_torch_benchmarks.py. Made-with: Cursor
…slang#782) Replace direct DescriptorMarshall construction with indirect triggers: reduce_type error via func.map(tex=(0,)), init error by passing a DescriptorHandle to a function whose module lacks slangpy import. Made-with: Cursor
Replace direct TensorMarshall construction with indirect triggers: - Grad dtype mismatch errors via Tensor.with_grads() + function call - Writable check via read-only tensor with grad_in + function call - Properties test via pack() instead of direct construction - Rename create_tensor_marshall test for consistency Made-with: Cursor
…ackward pass (shader-slang#782) - Use diff_pair() wrapper in 6 tests that used NativeTorchTensorDiffPair with is_input=True (the default) - Rewrite is_input=False test as actual backward pass test that triggers output diff pairs naturally through autograd - Rewrite grad-only factory test to use diff_pair(None, grad) - Add comment to test_slangtype_repr_no_reflection explaining why NativeSlangType is constructed directly Made-with: Cursor
…hader-slang#782) Use pack(module, diff_pair(primal, grad)) which internally constructs a BindContext and calls build_shader_object, instead of constructing these native objects directly. Made-with: Cursor
The NotImplementedError in TorchTensorMarshall.build_shader_object is an intentional stub — the C++ autograd path handles gradients directly and bypasses build_shader_object entirely. No value in covering a single raise line for a path that was never needed. Made-with: Cursor
shader-slang#782) Line numbers in the target source files will drift over time, making these references misleading rather than helpful. Made-with: Cursor
…PackedArg properties (shader-slang#782) - point_to shape/element_stride mismatch errors - Indexing: illegal argument type, too many indices - copy_from_numpy non-contiguous source rejection - clear() with explicit command encoder - is_contiguous singleton dimension skip - maybe_pad_data float3 padding (skips: stride=12 on current hw) - PackedArg .python/.shader_object/.python_object property access Made-with: Cursor
…der-slang#782) Move tests from test_torch_marshall_gaps.py into test_torchintegration.py (factory paths, properties, error guards, nanobind bindings) and test_torch_autograd_workflows.py (backward pass DiffPair output). Broaden parametrization from CUDA-only to DEVICE_TYPES and switch dispatch tests to get_torch_device() for Vulkan interop support. Made-with: Cursor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds extensive pytest coverage (many new CUDA/PyTorch tests) across torch integration, buffer/tensor marshalling, NDBuffer/BufferView, image loading, module/function reflection, and error paths. Introduces C++ changes to track an interop CUDA stream, allocate/keepalive interop device buffers, and wait for GPU submits; ensures image-loaded arrays are C-contiguous. Changes
Sequence Diagram(s)sequenceDiagram
participant PyTest as Python Test
participant TorchBridge as Torch Bridge
participant CallCtx as CallContext
participant NativeCall as NativeCallData
participant Device as GPU Device
PyTest->>TorchBridge: call function with torch.Tensor (may be CUDA)
TorchBridge->>CallCtx: mark_interop_cuda_stream(current PyTorch CUDA stream)
TorchBridge->>NativeCall: prepare marshall, allocate interop buffer if needed
NativeCall->>Device: submit_command_buffer(using opts.cuda_stream or CallCtx.interop_cuda_stream)
Device-->>NativeCall: submit_id (non-zero)
NativeCall->>Device: wait_for_submit(submit_id) [if readback required]
Device-->>PyTest: results returned (tensor/ndbuffer)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jhelferty-nv
changed the title
Made-with: Cursor # Conflicts: # slangpy/tests/slangpy_tests/test_torch_autograd_workflows.py
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
slangpy/tests/slangpy_tests/test_return_types.py (1)
211-214: Consider asserting the full matrix result, not only two elements.A full-array comparison would better catch accidental writes in padding/unpadding paths.
Proposed assertion refinement
modify_mat(v) result = v.value.to_numpy() - assert abs(result[0][0] - 11.0) < 1e-5 - assert abs(result[1][1] - 5.0) < 1e-5 + expected = orig.copy() + expected[0][0] += 10.0 + np.testing.assert_allclose(result, expected, rtol=0.0, atol=1e-5)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_return_types.py` around lines 211 - 214, The test currently checks only two elements of the matrix returned from v.value.to_numpy(), which can miss padding/unpadding bugs; update the assertions to compare the entire numpy array returned by to_numpy() against the expected full matrix (e.g., build the expected 2x2 matrix and use a full-array comparison such as numpy.testing.assert_allclose or an elementwise allclose assertion) so the test fails on any unexpected differences rather than only at [0][0] and [1][1].slangpy/tests/slangpy_tests/test_tensorview.py (1)
279-293: Consolidate duplicated slangpy.Tensor copy test.
test_tensorview_copy_tensoroverlaps with existingtest_tensorview_copy_slangpy_tensor(same setup/path/assertion intent). Keeping both adds maintenance and CI time without new behavioral coverage; consider merging into one canonical test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_tensorview.py` around lines 279 - 293, The two tests test_tensorview_copy_tensor and test_tensorview_copy_slangpy_tensor duplicate setup and assertions for module.copy_tensorview (using Tensor.from_numpy, Tensor.zeros and output_tensor.to_numpy), so remove one and keep a single canonical test to cover the behavior across DEVICE_TYPES; specifically, delete test_tensorview_copy_tensor and ensure test_tensorview_copy_slangpy_tensor parametrizes DEVICE_TYPES, constructs input via Tensor.from_numpy, output via Tensor.zeros, calls module.copy_tensorview(input, output) and asserts np.allclose(result, data, atol=1e-5) so no coverage is lost.slangpy/tests/slangpy_tests/test_shape.py (1)
161-161: Consider adding type annotations on the new test method signatures.Both new methods currently leave arguments unannotated. If you’re enforcing the repository-wide rule consistently, annotate
selfand return type.♻️ Suggested update
- def test_equality_incompatible_type(self): + def test_equality_incompatible_type(self: "TestShapeOperations") -> None: """Test equality returns False for non-Shape/non-list types.""" s = Shape([1, 2, 3]) assert s != "not a shape" assert s != 42 assert s != 3.14 ... - def test_bytes_property(self): + def test_bytes_property(self: "TestSignatureBuilder") -> None: """SignatureBuilder.bytes returns raw bytes matching str encoding.""" sb = SignatureBuilder()As per coding guidelines
**/*.py: Use type annotations for all Python function arguments.Also applies to: 463-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_shape.py` at line 161, The test method test_equality_incompatible_type (and the other new test at the referenced location) lack type annotations; update their signatures to annotate self as "self: TestCase" (or appropriate test class type) and add a return annotation of "-> None" so both methods read e.g. def test_equality_incompatible_type(self: TestCase) -> None:, ensuring consistency with the repository's function argument annotations.slangpy/tests/slangpy_tests/test_textures.py (1)
620-620: Add explicit type annotations to the new helper/test function signatures.These new Python function arguments are currently untyped, which breaks the repository typing rule.
♻️ Proposed fix
-def _create_simple_texture(device, tex_type, width=16, height=16, depth=16, array_length=1): +def _create_simple_texture( + device: object, + tex_type: TextureType, + width: int = 16, + height: int = 16, + depth: int = 16, + array_length: int = 1, +) -> Texture: @@ -def test_get_texture_shape(device_type, tex_type, expected_shape): +def test_get_texture_shape( + device_type: DeviceType, tex_type: TextureType, expected_shape: tuple[int, ...] +) -> None: @@ -def test_get_texture_shape_array_types(device_type): +def test_get_texture_shape_array_types(device_type: DeviceType) -> None:As per coding guidelines:
**/*.py: Use type annotations for all Python function arguments.Also applies to: 652-652, 661-661
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_textures.py` at line 620, Add explicit type annotations to the new test helper signatures: annotate _create_simple_texture parameters as device: Any, tex_type: Any, width: int = 16, height: int = 16, depth: int = 16, array_length: int = 1 and give the function an explicit return type (e.g., Any or the concrete texture type used in tests). Import Any from typing if needed. Apply the same pattern to the other new helper/test functions in the same file that lack annotations so all function arguments and return types are explicitly typed.slangpy/tests/slangpy_tests/test_bridge_fallback_gaps.py (1)
98-103: Add non-contiguous/storage-offset round-trip cases for copy helpers.Current success-path tests cover contiguous tensors only. Adding transposed/sliced tensors would better protect documented fallback behavior for non-contiguous layouts.
♻️ Suggested test additions
class TestCopyToBuffer: @@ `@pytest.mark.skipif`(not torch.cuda.is_available(), reason="CUDA not available") def test_copy_round_trip(self): src = torch.tensor([1.0, 2.0, 3.0], device="cuda", dtype=torch.float32) dest = torch.zeros(3, device="cuda", dtype=torch.float32) bf.copy_to_buffer(src, dest.data_ptr(), dest.numel() * dest.element_size()) assert torch.allclose(src, dest) + + `@pytest.mark.skipif`(not torch.cuda.is_available(), reason="CUDA not available") + def test_copy_round_trip_non_contiguous(self): + base = torch.arange(12, device="cuda", dtype=torch.float32).reshape(3, 4) + src = base.t() # non-contiguous view + dest = torch.empty_like(src.contiguous()) + bf.copy_to_buffer(src, dest.data_ptr(), dest.numel() * dest.element_size()) + assert torch.allclose(src, dest) @@ class TestCopyFromBuffer: @@ `@pytest.mark.skipif`(not torch.cuda.is_available(), reason="CUDA not available") def test_copy_round_trip(self): src = torch.tensor([4.0, 5.0, 6.0], device="cuda", dtype=torch.float32) dest = torch.zeros(3, device="cuda", dtype=torch.float32) bf.copy_from_buffer(dest, src.data_ptr(), src.numel() * src.element_size()) assert torch.allclose(src, dest) + + `@pytest.mark.skipif`(not torch.cuda.is_available(), reason="CUDA not available") + def test_copy_round_trip_storage_offset_view(self): + src = torch.arange(8, device="cuda", dtype=torch.float32)[2:] # storage_offset > 0 + dest_base = torch.zeros(10, device="cuda", dtype=torch.float32) + dest = dest_base[1:7] # non-zero storage offset + bf.copy_from_buffer(dest, src.data_ptr(), src.numel() * src.element_size()) + assert torch.allclose(src, dest)Based on learnings: "Test files must run torch bridge tests in both native and fallback modes via fixture, covering CPU/CUDA tensors, various dtypes, non-contiguous layouts, transposed tensors, and storage offsets".
Also applies to: 119-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_bridge_fallback_gaps.py` around lines 98 - 103, The test_copy_round_trip only exercises contiguous tensors; add cases exercising non-contiguous layouts and storage-offsets so bf.copy_to_buffer and the corresponding copy-back logic are validated for transposed/sliced views and tensors with non-zero storage_offset. Update test_copy_round_trip (and the similar case at lines 119-123) to parametrize or add subtests that create src as transposed (e.g., src.t()), sliced strided views, and tensors made via offsets (e.g., a slice of a larger tensor) and then perform bf.copy_to_buffer(src, dest.data_ptr(), src.numel() * src.element_size()) and assert torch.allclose(src, dest.view_as(src) or dest_contiguous_view) for each layout; ensure the existing test fixture that runs bridge tests in both native and fallback modes covers these new cases so both paths are exercised.slangpy/tests/slangpy_tests/test_tensor_marshall_gaps.py (1)
169-173: Make thisxfailstrict so fixed bugs don’t get silently ignored.Using
strict=Trueturns unexpected passes into a visible signal to remove/update the marker.Suggested tweak
`@pytest.mark.xfail`( reason="TensorMarshall.build_shader_object derivative path references 'primal' field " "that doesn't exist on the shader object type; likely needs DiffTensor infrastructure", raises=RuntimeError, + strict=True, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_tensor_marshall_gaps.py` around lines 169 - 173, The xfail marker on the test in test_tensor_marshall_gaps.py should be made strict so unexpected passes fail the test suite: update the pytest.mark.xfail decorator invocation used on that test to include strict=True (i.e., pytest.mark.xfail(..., strict=True)) while keeping the existing reason and raises arguments intact so the test will surface when the underlying bug is fixed.slangpy/tests/slangpy_tests/test_load_image.py (1)
97-103: Rename unused variable to indicate intentional discard.The
expectedvariable is unpacked but never used since this test only validates shape properties for 2-channel images.Suggested fix
def test_load_two_channel(device, rg_image): """Load a 2-channel image and verify it produces float2 tensor.""" - path, expected = rg_image + path, _ = rg_image tensor = spy.Tensor.load_from_image(device, path) result = tensor.to_numpy() assert result.ndim == 3 assert result.shape[2] == 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_load_image.py` around lines 97 - 103, The test_load_two_channel function unpacks rg_image into path and expected but never uses expected; rename the unused variable (expected) to _expected or simply _ to indicate intentional discard. Update the tuple unpacking in test_load_two_channel (where it currently reads "path, expected") to use "path, _expected" or "path, _" so the test intent is clear and linter warnings about unused variables are avoided.slangpy/tests/slangpy_tests/test_value_torch_pipeline.py (2)
207-248: Use raw strings for regex patterns.Same linter warning as in other test files.
Suggested fix
`@pytest.mark.parametrize`("device_type", CUDA_TYPES) def test_scalar_reduce_type_error(device_type: DeviceType): """ScalarMarshall.reduce_type raises for dimensions > 0.""" from slangpy.bindings.typeregistry import PYTHON_TYPES device = helpers.get_device(device_type) func = helpers.create_function_from_module(device, "add_scalar", ADD_SCALAR_SHADER) layout = func.module.layout marshall = PYTHON_TYPES[float](layout, 1.0) - with pytest.raises(ValueError, match="[Cc]annot reduce"): + with pytest.raises(ValueError, match=r"[Cc]annot reduce"): marshall.reduce_type(None, 1)with pytest.raises(ValueError, match=r"[Cc]annot reduce"): marshall.reduce_type(None, 2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_value_torch_pipeline.py` around lines 207 - 248, The pytest.raises calls in test_scalar_reduce_type_error and test_vector_reduce_type use normal string regex literals for the match argument; change those match arguments to raw string literals (prefix with r) so linter regex warnings go away—i.e., update the match="[Cc]annot reduce" occurrences passed to pytest.raises in these tests (where marshall.reduce_type is invoked) to match=r"[Cc]annot reduce".
158-178: Add assertion or note the test purpose is exercising code path only.In
test_pack_float2_vector, theresultvariable is assigned but never used. If the test's purpose is solely to exerciseVectorMarshall.build_shader_objectwithout validating output, consider either:
- Adding a minimal assertion (e.g.,
assert result is None or result is not None)- Prefixing with
_to indicate intentional discardIn
test_pack_scalar_value, the test correctly assertspacked_scalar is not None.Suggested fix for consistency
`@pytest.mark.parametrize`("device_type", CUDA_TYPES) def test_pack_float2_vector(device_type: DeviceType): """pack() a float2 vector value, exercising VectorMarshall.build_shader_object.""" device = helpers.get_device(device_type) func = helpers.create_function_from_module(device, "scale_vec", SCALE_VECTOR_SHADER) v = slangpy.math.float2(3.0, 4.0) packed = pack(func.module, v) out_buf = slangpy.types.NDBuffer.zeros(device, shape=(1,), dtype=float) - result = func(packed, 2.0, out_buf) + func(packed, 2.0, out_buf) # Exercise VectorMarshall.build_shader_object + # Output validation not needed; test exercises the code path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_value_torch_pipeline.py` around lines 158 - 178, The test test_pack_float2_vector assigns result = func(packed, 2.0, out_buf) but never uses it; either assert a trivial condition on result or mark it as intentionally discarded to avoid unused-variable warnings and clarify intent. Update test_pack_float2_vector to either add a minimal assertion (e.g., assert result is not None or assert result is None depending on expected behavior) or rename the variable to _result to indicate the call is only exercising VectorMarshall.build_shader_object via pack(func.module, v) and func invocation.slangpy/tests/slangpy_tests/test_torchintegration.py (2)
914-921: Use raw string for regex pattern.Same issue as above - the character classes work but should use a raw string prefix.
Suggested fix
def test_torch_cpu_tensor_rejected(device_type: DeviceType): """Non-CUDA torch tensors are rejected by write_shader_cursor_pre_dispatch.""" device = helpers.get_torch_device(device_type) func = helpers.create_function_from_module(device, "identity", IDENTITY_SRC) cpu_tensor = torch.tensor([1.0, 2.0], dtype=torch.float32) - with pytest.raises(Exception, match="[Cc][Uu][Dd][Aa]"): + with pytest.raises(Exception, match=r"[Cc][Uu][Dd][Aa]"): func(cpu_tensor)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 914 - 921, The pytest.raises call in test_torch_cpu_tensor_rejected uses a non-raw regex string for match="[Cc][Uu][Dd][Aa]"; change it to a raw string (match=r"[Cc][Uu][Dd][Aa]") so backslashes and escapes in the regex are interpreted correctly by Python; update the match argument in test_torch_cpu_tensor_rejected accordingly.
874-891: Use raw strings for regex patterns with character classes.The
match=patterns contain character classes[Uu]that work correctly but trigger linter warnings. Using raw strings makes the intent clearer and is a Python best practice for regex patterns.Suggested fix
`@pytest.mark.parametrize`("device_type", DEVICE_TYPES) def test_factory_unsupported_torch_dtype_raises(device_type: DeviceType): """Passing a tensor with unsupported dtype raises ValueError.""" layout = _get_layout(device_type) t = torch.tensor([1.0 + 2.0j], dtype=torch.complex64, device="cuda") - with pytest.raises(ValueError, match="[Uu]nsupported"): + with pytest.raises(ValueError, match=r"[Uu]nsupported"): ttm.create_torch_tensor_marshall(layout, t) `@pytest.mark.parametrize`("device_type", DEVICE_TYPES) def test_diffpair_factory_unsupported_dtype_raises(device_type: DeviceType): """DiffPair factory raises for unsupported torch dtype.""" layout = _get_layout(device_type) primal = torch.tensor([1.0 + 2.0j], dtype=torch.complex64, device="cuda") grad = torch.tensor([0.0 + 0.0j], dtype=torch.complex64, device="cuda") pair = diff_pair(primal, grad) - with pytest.raises(ValueError, match="[Uu]nsupported"): + with pytest.raises(ValueError, match=r"[Uu]nsupported"): ttm.create_torch_tensor_marshall(layout, pair)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 874 - 891, The pytest assertions in test_factory_unsupported_torch_dtype_raises and test_diffpair_factory_unsupported_dtype_raises use match="[Uu]nsupported" which triggers linter warnings; update both calls to use raw string literals for the regex (e.g. change match="[Uu]nsupported" to match=r"[Uu]nsupported") so the regex is a raw string while keeping the same pattern and behavior in ttm.create_torch_tensor_marshall tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@slangpy/tests/slangpy_tests/test_buffer_views.py`:
- Around line 329-343: The test test_copy_from_torch_cpu_fallback is only
skipping on macOS but still calls helpers.get_device(DeviceType.cuda), which
fails when CUDA is not available on non-macOS runners; change the test to detect
CUDA availability after importing torch and skip if CUDA isn’t available (e.g.,
call torch.cuda.is_available() and pytest.skip when False) or catch the error
from helpers.get_device(DeviceType.cuda) and pytest.skip, so the test only
proceeds when a CUDA device can actually be acquired.
In `@slangpy/tests/slangpy_tests/test_raw_dispatch.py`:
- Around line 218-220: The nested callback function writer has untyped
parameters; add type annotations for its arguments (cursor and k_value) in the
writer definition (e.g., def writer(cursor: CursorType, k_value: int):),
importing or aliasing the appropriate CursorType from the test utilities or
falling back to typing.Any if a concrete type isn't exported; ensure the
annotation follows the repo's typing style and update imports if needed.
In `@slangpy/tests/slangpy_tests/test_torch_dtypes.py`:
- Around line 18-67: The tests lack Python type annotations on fixture and test
function arguments; update the fixture and tests to add types: annotate the
fixture function device() with a return type (e.g., -> spy.DeviceType or the
correct device type used by get_device), and annotate each test argument — e.g.,
test_torch_output_int32(device: spy.DeviceType), test_torch_output_int64(device:
spy.DeviceType), test_torch_output_float64(device: spy.DeviceType), and
test_torch_output_rare_scalar(device: spy.DeviceType, slang_func: str,
torch_dtype: torch.dtype) — so all function arguments (device, slang_func,
torch_dtype) have explicit type annotations matching the repo typing
conventions.
---
Nitpick comments:
In `@slangpy/tests/slangpy_tests/test_bridge_fallback_gaps.py`:
- Around line 98-103: The test_copy_round_trip only exercises contiguous
tensors; add cases exercising non-contiguous layouts and storage-offsets so
bf.copy_to_buffer and the corresponding copy-back logic are validated for
transposed/sliced views and tensors with non-zero storage_offset. Update
test_copy_round_trip (and the similar case at lines 119-123) to parametrize or
add subtests that create src as transposed (e.g., src.t()), sliced strided
views, and tensors made via offsets (e.g., a slice of a larger tensor) and then
perform bf.copy_to_buffer(src, dest.data_ptr(), src.numel() *
src.element_size()) and assert torch.allclose(src, dest.view_as(src) or
dest_contiguous_view) for each layout; ensure the existing test fixture that
runs bridge tests in both native and fallback modes covers these new cases so
both paths are exercised.
In `@slangpy/tests/slangpy_tests/test_load_image.py`:
- Around line 97-103: The test_load_two_channel function unpacks rg_image into
path and expected but never uses expected; rename the unused variable (expected)
to _expected or simply _ to indicate intentional discard. Update the tuple
unpacking in test_load_two_channel (where it currently reads "path, expected")
to use "path, _expected" or "path, _" so the test intent is clear and linter
warnings about unused variables are avoided.
In `@slangpy/tests/slangpy_tests/test_return_types.py`:
- Around line 211-214: The test currently checks only two elements of the matrix
returned from v.value.to_numpy(), which can miss padding/unpadding bugs; update
the assertions to compare the entire numpy array returned by to_numpy() against
the expected full matrix (e.g., build the expected 2x2 matrix and use a
full-array comparison such as numpy.testing.assert_allclose or an elementwise
allclose assertion) so the test fails on any unexpected differences rather than
only at [0][0] and [1][1].
In `@slangpy/tests/slangpy_tests/test_shape.py`:
- Line 161: The test method test_equality_incompatible_type (and the other new
test at the referenced location) lack type annotations; update their signatures
to annotate self as "self: TestCase" (or appropriate test class type) and add a
return annotation of "-> None" so both methods read e.g. def
test_equality_incompatible_type(self: TestCase) -> None:, ensuring consistency
with the repository's function argument annotations.
In `@slangpy/tests/slangpy_tests/test_tensor_marshall_gaps.py`:
- Around line 169-173: The xfail marker on the test in
test_tensor_marshall_gaps.py should be made strict so unexpected passes fail the
test suite: update the pytest.mark.xfail decorator invocation used on that test
to include strict=True (i.e., pytest.mark.xfail(..., strict=True)) while keeping
the existing reason and raises arguments intact so the test will surface when
the underlying bug is fixed.
In `@slangpy/tests/slangpy_tests/test_tensorview.py`:
- Around line 279-293: The two tests test_tensorview_copy_tensor and
test_tensorview_copy_slangpy_tensor duplicate setup and assertions for
module.copy_tensorview (using Tensor.from_numpy, Tensor.zeros and
output_tensor.to_numpy), so remove one and keep a single canonical test to cover
the behavior across DEVICE_TYPES; specifically, delete
test_tensorview_copy_tensor and ensure test_tensorview_copy_slangpy_tensor
parametrizes DEVICE_TYPES, constructs input via Tensor.from_numpy, output via
Tensor.zeros, calls module.copy_tensorview(input, output) and asserts
np.allclose(result, data, atol=1e-5) so no coverage is lost.
In `@slangpy/tests/slangpy_tests/test_textures.py`:
- Line 620: Add explicit type annotations to the new test helper signatures:
annotate _create_simple_texture parameters as device: Any, tex_type: Any, width:
int = 16, height: int = 16, depth: int = 16, array_length: int = 1 and give the
function an explicit return type (e.g., Any or the concrete texture type used in
tests). Import Any from typing if needed. Apply the same pattern to the other
new helper/test functions in the same file that lack annotations so all function
arguments and return types are explicitly typed.
In `@slangpy/tests/slangpy_tests/test_torchintegration.py`:
- Around line 914-921: The pytest.raises call in test_torch_cpu_tensor_rejected
uses a non-raw regex string for match="[Cc][Uu][Dd][Aa]"; change it to a raw
string (match=r"[Cc][Uu][Dd][Aa]") so backslashes and escapes in the regex are
interpreted correctly by Python; update the match argument in
test_torch_cpu_tensor_rejected accordingly.
- Around line 874-891: The pytest assertions in
test_factory_unsupported_torch_dtype_raises and
test_diffpair_factory_unsupported_dtype_raises use match="[Uu]nsupported" which
triggers linter warnings; update both calls to use raw string literals for the
regex (e.g. change match="[Uu]nsupported" to match=r"[Uu]nsupported") so the
regex is a raw string while keeping the same pattern and behavior in
ttm.create_torch_tensor_marshall tests.
In `@slangpy/tests/slangpy_tests/test_value_torch_pipeline.py`:
- Around line 207-248: The pytest.raises calls in test_scalar_reduce_type_error
and test_vector_reduce_type use normal string regex literals for the match
argument; change those match arguments to raw string literals (prefix with r) so
linter regex warnings go away—i.e., update the match="[Cc]annot reduce"
occurrences passed to pytest.raises in these tests (where marshall.reduce_type
is invoked) to match=r"[Cc]annot reduce".
- Around line 158-178: The test test_pack_float2_vector assigns result =
func(packed, 2.0, out_buf) but never uses it; either assert a trivial condition
on result or mark it as intentionally discarded to avoid unused-variable
warnings and clarify intent. Update test_pack_float2_vector to either add a
minimal assertion (e.g., assert result is not None or assert result is None
depending on expected behavior) or rename the variable to _result to indicate
the call is only exercising VectorMarshall.build_shader_object via
pack(func.module, v) and func invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 764d58db-5f86-456f-9ff8-1a7f153ab973
📒 Files selected for processing (26)
slangpy/builtin/diffpair.pyslangpy/tests/slangpy_tests/test_bridge_fallback_gaps.pyslangpy/tests/slangpy_tests/test_buffer_views.pyslangpy/tests/slangpy_tests/test_buffers.pyslangpy/tests/slangpy_tests/test_descriptor_marshall.pyslangpy/tests/slangpy_tests/test_differential_function_call.pyslangpy/tests/slangpy_tests/test_diffpair_type.pyslangpy/tests/slangpy_tests/test_difftensorview.pyslangpy/tests/slangpy_tests/test_load_image.pyslangpy/tests/slangpy_tests/test_modules.pyslangpy/tests/slangpy_tests/test_ndbuffer.pyslangpy/tests/slangpy_tests/test_raw_dispatch.pyslangpy/tests/slangpy_tests/test_return_types.pyslangpy/tests/slangpy_tests/test_shape.pyslangpy/tests/slangpy_tests/test_tensor.pyslangpy/tests/slangpy_tests/test_tensor_marshall_gaps.pyslangpy/tests/slangpy_tests/test_tensorview.pyslangpy/tests/slangpy_tests/test_textures.pyslangpy/tests/slangpy_tests/test_torch_autograd_workflows.pyslangpy/tests/slangpy_tests/test_torch_benchmarks.pyslangpy/tests/slangpy_tests/test_torch_dtypes.pyslangpy/tests/slangpy_tests/test_torch_mixed.pyslangpy/tests/slangpy_tests/test_torchintegration.pyslangpy/tests/slangpy_tests/test_tostring.pyslangpy/tests/slangpy_tests/test_value_torch_pipeline.pyslangpy/types/common.py
💤 Files with no reviewable changes (1)
- slangpy/tests/slangpy_tests/test_differential_function_call.py
Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
slangpy/tests/slangpy_tests/test_difftensorview.py (1)
51-65: Consolidate duplicate copy coverage and avoid cache-coupled device selectionThis test overlaps heavily with
test_difftensorview_copy_slangpy_tensor(Lines 136-147). Also, Line 55 reacquires a device instead of usingmodule.device, which adds unnecessary cache-coupling risk.♻️ Suggested minimal cleanup in this segment
`@pytest.mark.parametrize`("device_type", DEVICE_TYPES) def test_difftensorview_copy_tensor(device_type: DeviceType): """Test copy_difftensorview with slangpy.Tensor — exercises tensorcommon resolve_types for DiffTensorView.""" module = load_module(device_type) - device = helpers.get_device(type=device_type) data = np.array([1.0, 2.0, 3.0, 4.0, 5.0], dtype=np.float32) - input_tensor = Tensor.from_numpy(device, data) - output_tensor = Tensor.zeros(device, (5,), "float") + input_tensor = Tensor.from_numpy(module.device, data) + output_tensor = Tensor.zeros(module.device, (5,), "float")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_difftensorview.py` around lines 51 - 65, The test test_difftensorview_copy_tensor duplicates coverage from test_difftensorview_copy_slangpy_tensor and unnecessarily reacquires a device via helpers.get_device(device_type); update the test to reuse the device provided by load_module (use module.device) to avoid cache-coupled device selection and remove or consolidate redundant assertions with test_difftensorview_copy_slangpy_tensor; specifically, in test_difftensorview_copy_tensor replace the helpers.get_device call with device = module.device and keep the same tensor creation/copy/assert logic or delete this test and ensure the remaining test covers both Tensor and slangpy.Tensor copy paths across DEVICE_TYPES.slangpy/tests/slangpy_tests/test_torchintegration.py (2)
981-983: Avoid broadExceptionin guard-path tests.Use a specific exception type (
ValueError,TypeError, orSlangPyError) so these tests fail if the wrong error path triggers.✅ Suggested diff (example)
- with pytest.raises(Exception, match="shape"): + with pytest.raises((ValueError, TypeError), match="shape"): func(bad) @@ - with pytest.raises(Exception, match=r"[Cc][Uu][Dd][Aa]"): + with pytest.raises((ValueError, TypeError), match=r"[Cc][Uu][Dd][Aa]"): func(cpu_tensor)As per coding guidelines: "Python-layer errors should raise standard Python exceptions (ValueError, TypeError, or SlangPyError)".
Also applies to: 992-993
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 981 - 983, Replace the broad pytest.raises(Exception, ...) assertions in the torch integration tests with a specific exception type so incorrect error paths are caught; update the two occurrences where the test calls pytest.raises(Exception, match="shape") around the call to func(bad) (and the similar case at lines 992-993) to use the appropriate specific exception (ValueError, TypeError, or SlangPyError) that the code is expected to raise for invalid tensor shapes.
794-810: Strengthen assertions in new DiffPair execution tests.
assert result is not Noneis too weak for regression detection. Assert expected value/shape (and gradient where relevant) to ensure these paths verify behavior, not just non-crash execution.Based on learnings: "Applies to slangpy/tests/slangpy_tests/test_torchintegration.py : End-to-end torch integration tests must cover: scalars/vectors/arrays/generics, extra vectorization dims, return modes (return/pass/out), autograd forward+backward, cache reuse, and structured types".
Also applies to: 813-823
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 794 - 810, The test test_diffpair_read_signature uses a weak assertion; strengthen it by asserting the actual tensor value and shape returned by func when passed diff_pair(primal, grad) and also verify autograd behavior by calling backward on a suitable scalar or sum of result and asserting that primal.grad (or the paired grad) matches expected gradients; update assertions in the related test block (the similar test around the next diff pair section) likewise to check result.shape, numeric contents (e.g., squared values for "square"), and that gradients equal the expected tensor (ones), referencing test_diffpair_read_signature, diff_pair, func, primal, grad, and result to locate and modify the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@slangpy/tests/slangpy_tests/test_torchintegration.py`:
- Line 950: The tests in test_torchintegration.py use pytest.raises(...,
match="...") with regex patterns that should be raw strings; update the
pytest.raises calls (the one shown and the other occurrences around the
pytest.raises usages at the nearby assertions) to use raw string literals
(prefix the pattern with r, e.g., r"[Uu]nsupported") so regex metacharacters are
interpreted correctly and to satisfy Ruff RUF043; locate the pytest.raises calls
in the test_torchintegration.py file (the test function(s) invoking
pytest.raises) and change their match arguments to raw strings.
---
Nitpick comments:
In `@slangpy/tests/slangpy_tests/test_difftensorview.py`:
- Around line 51-65: The test test_difftensorview_copy_tensor duplicates
coverage from test_difftensorview_copy_slangpy_tensor and unnecessarily
reacquires a device via helpers.get_device(device_type); update the test to
reuse the device provided by load_module (use module.device) to avoid
cache-coupled device selection and remove or consolidate redundant assertions
with test_difftensorview_copy_slangpy_tensor; specifically, in
test_difftensorview_copy_tensor replace the helpers.get_device call with device
= module.device and keep the same tensor creation/copy/assert logic or delete
this test and ensure the remaining test covers both Tensor and slangpy.Tensor
copy paths across DEVICE_TYPES.
In `@slangpy/tests/slangpy_tests/test_torchintegration.py`:
- Around line 981-983: Replace the broad pytest.raises(Exception, ...)
assertions in the torch integration tests with a specific exception type so
incorrect error paths are caught; update the two occurrences where the test
calls pytest.raises(Exception, match="shape") around the call to func(bad) (and
the similar case at lines 992-993) to use the appropriate specific exception
(ValueError, TypeError, or SlangPyError) that the code is expected to raise for
invalid tensor shapes.
- Around line 794-810: The test test_diffpair_read_signature uses a weak
assertion; strengthen it by asserting the actual tensor value and shape returned
by func when passed diff_pair(primal, grad) and also verify autograd behavior by
calling backward on a suitable scalar or sum of result and asserting that
primal.grad (or the paired grad) matches expected gradients; update assertions
in the related test block (the similar test around the next diff pair section)
likewise to check result.shape, numeric contents (e.g., squared values for
"square"), and that gradients equal the expected tensor (ones), referencing
test_diffpair_read_signature, diff_pair, func, primal, grad, and result to
locate and modify the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c48ddbab-8a3b-4046-a926-b5c5e60eab5b
📒 Files selected for processing (3)
slangpy/tests/slangpy_tests/test_difftensorview.pyslangpy/tests/slangpy_tests/test_torch_autograd_workflows.pyslangpy/tests/slangpy_tests/test_torchintegration.py
🚧 Files skipped from review as they are similar to previous changes (1)
- slangpy/tests/slangpy_tests/test_torch_autograd_workflows.py
Contributor
Author
Coverage detailsAll coverage data collected on a machine with an NVIDIA GeForce GTX 960 (Maxwell, SM 5.2). Some code paths are unreachable on this hardware — see "Not testable on current hardware" and "Follow-up tasks" below. Overall summary (weighted by LOC)
Python — implementation files (non-test, non-benchmark)
C++ — slangpy_ext/utils (.cpp files, raw gcov)
Header coverage (raw gcov, largest TU shown):
C++ — slangpy_ext/device (.cpp files, raw gcov)Nanobind wrappers around the sgl device layer. Many files show different line counts due to upstream changes.
C++ — slangpy_torch
Refactoring principlesAll tests were audited for direct use of
Remaining direct
Dead code identified
Bugs found
Remaining gaps — skipped with justification
Follow-up tasks (for other environments)
|
Made-with: Cursor
Tests that directly create torch tensors on CUDA now skip when CUDA is not available. Fixes failures on non-CUDA runners (e.g. macOS). Made-with: Cursor
Prefix match patterns containing regex metacharacters with r to satisfy Ruff RUF043 and make regex intent explicit. Made-with: Cursor
DescriptorHandle type resolution is broken (resolves to StructuredBuffer<Unknown> instead of the actual descriptor type), causing all four tests to fail with ResolveException or wrong error messages. Mark as strict xfail so they surface when shader-slang#922 is fixed. Made-with: Cursor
Made-with: Cursor # Conflicts: # slangpy/tests/slangpy_tests/test_torchintegration.py
Replace non-ASCII glyphs (arrows, em-dashes) with ASCII equivalents to pass check-ascii-source pre-commit hook. Call func(pair) twice in test_diffpair_get_shape_grad_only to exercise the second-dispatch regression path from shader-slang#929. Apply clang-format. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/slangpy_ext/utils/slangpy.cpp (1)
977-1010:⚠️ Potential issue | 🟡 MinorUse ASCII-only text in the new execution comments.
check-ascii-sourceis failing on this hunk. The↔and—characters in Lines 978-1007 need ASCII replacements before this can merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/slangpy_ext/utils/slangpy.cpp` around lines 977 - 1010, The comment text contains non-ASCII characters (the bidirectional arrow and em dash) that trigger check-ascii-source; replace the non-ASCII characters in the new execution comments surrounding the submit/cleanup logic (near temp_command_encoder, submit_command_buffer, command_encoder, read_back, wait_for_submit and DeviceType::cuda) with ASCII equivalents (e.g., use "<->" or "->" for ↔ and "-" for —) so all comments are plain ASCII; keep the explanatory intent and punctuation but only swap those characters in the comment strings.src/slangpy_ext/utils/slangpytorchtensor.cpp (1)
627-632:⚠️ Potential issue | 🟠 MajorUse the same CUDA stream for the memset and the recorded interop sync.
When
context->cuda_stream()is unset, Line 631 enqueuesmemset_device_async()on the default stream, but Line 632 records PyTorch's current stream.submit_command_buffer()will then synchronize graphics work against a different stream than the one that actually zeroed the buffer.Suggested fix
void* cuda_ptr = interop_buffer->cuda_memory(); if (cuda_ptr && buffer_size > 0) { - CUstream stream = context->cuda_stream().is_valid() - ? reinterpret_cast<CUstream>(context->cuda_stream().value()) - : nullptr; - cuda::memset_device_async(static_cast<uint8_t*>(cuda_ptr), 0, buffer_size, stream); - mark_interop_stream(info.device_index); + void* stream_ptr = context->cuda_stream().is_valid() + ? reinterpret_cast<void*>(context->cuda_stream().value()) + : TorchBridge::instance().get_current_cuda_stream(info.device_index); + cuda::memset_device_async( + static_cast<uint8_t*>(cuda_ptr), + 0, + buffer_size, + reinterpret_cast<CUstream>(stream_ptr) + ); + context->mark_interop_cuda_stream( + NativeHandle(rhi::NativeHandle(rhi::NativeHandleType::CUstream, reinterpret_cast<uint64_t>(stream_ptr))) + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/slangpy_ext/utils/slangpytorchtensor.cpp` around lines 627 - 632, The memset is enqueued on a stream computed from context->cuda_stream(), but mark_interop_stream() records PyTorch's current stream separately so they may differ; change the code in slangpytorchtensor.cpp so you compute a single CUstream variable (use context->cuda_stream() when set, otherwise obtain PyTorch's current CUDA stream for info.device_index), pass that stream into cuda::memset_device_async(...) and ensure mark_interop_stream(...) records that same stream (e.g., add/use an overload or change mark_interop_stream to accept the CUstream or a wrapper) so the memset and the interop sync use the identical CUDA stream.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sgl/utils/slangpy.h`:
- Around line 430-441: The comments around mark_interop_cuda_stream and
interop_cuda_stream include non-ASCII punctuation (specifically the ↔ and —
characters); update those comment lines to use ASCII-only equivalents (e.g.,
replace ↔ with "<->" or "↔" -> "to" and replace — with "--" or a simple hyphen)
so that check-ascii-source passes while leaving the function names and
m_interop_cuda_stream usage unchanged.
In `@src/slangpy_ext/utils/slangpytorchtensor.cpp`:
- Around line 579-583: The comment block describing CUDA stream recording
contains non-ASCII punctuation (the ↔ arrow and the — em-dash); update that
comment to use ASCII-only characters (e.g. replace ↔ with <-> or <- -> and
replace — with -- or a single hyphen) in the comment that references exec(),
submit_command_buffer(), copy_to_buffer, and memset_device_async so
check-ascii-source passes.
- Around line 493-503: When offsets.is_tensorview and interop_buffer is present
the code rebuilds TensorViewData for a TensorView/DiffTensorView which are
CUDA-only; add a guard that rejects or skips the interop-buffer path for
TensorView types when the runtime/backend is not CUDA (i.e., detect backend from
the interop_buffer or runtime context and if not CUDA, do not populate
TensorViewData from interop_buffer and return an error or fallback), keeping the
existing TensorViewData population for true CUDA interop only; reference
offsets.is_tensorview, TensorViewData, interop_buffer, and the existing stride
logic (make_contiguous_strides, apply_broadcast_stride_zeroing,
binding->transform(), context->call_shape()) to locate and gate the change.
---
Outside diff comments:
In `@src/slangpy_ext/utils/slangpy.cpp`:
- Around line 977-1010: The comment text contains non-ASCII characters (the
bidirectional arrow and em dash) that trigger check-ascii-source; replace the
non-ASCII characters in the new execution comments surrounding the
submit/cleanup logic (near temp_command_encoder, submit_command_buffer,
command_encoder, read_back, wait_for_submit and DeviceType::cuda) with ASCII
equivalents (e.g., use "<->" or "->" for ↔ and "-" for —) so all comments are
plain ASCII; keep the explanatory intent and punctuation but only swap those
characters in the comment strings.
In `@src/slangpy_ext/utils/slangpytorchtensor.cpp`:
- Around line 627-632: The memset is enqueued on a stream computed from
context->cuda_stream(), but mark_interop_stream() records PyTorch's current
stream separately so they may differ; change the code in slangpytorchtensor.cpp
so you compute a single CUstream variable (use context->cuda_stream() when set,
otherwise obtain PyTorch's current CUDA stream for info.device_index), pass that
stream into cuda::memset_device_async(...) and ensure mark_interop_stream(...)
records that same stream (e.g., add/use an overload or change
mark_interop_stream to accept the CUstream or a wrapper) so the memset and the
interop sync use the identical CUDA stream.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5d0a818-5550-4f40-b965-96cc6ff0d5de
📒 Files selected for processing (4)
slangpy/tests/slangpy_tests/test_torchintegration.pysrc/sgl/utils/slangpy.hsrc/slangpy_ext/utils/slangpy.cppsrc/slangpy_ext/utils/slangpytorchtensor.cpp
Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
slangpy/tests/slangpy_tests/test_load_image.py (1)
105-112:⚠️ Potential issue | 🟡 MinorAssert two-channel pixel values, not just shape.
expectedis still unused, so channel-order/value regressions can pass undetected. Please compare full tensor contents here.Suggested patch
def test_load_two_channel(device, rg_image): """Load a 2-channel image and verify it produces float2 tensor.""" path, expected = rg_image tensor = spy.Tensor.load_from_image(device, path) result = tensor.to_numpy() assert result.ndim == 3 assert result.shape[2] == 2 + assert result.shape == expected.shape + assert np.allclose(result, expected, atol=1e-5)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_load_image.py` around lines 105 - 112, The test_load_two_channel currently only checks shape; update it to assert pixel values match the fixture by using the expected value from rg_image: after loading with spy.Tensor.load_from_image and converting to numpy (result), compare result to expected (or expected.astype/result dtype) using an elementwise comparison (e.g., numpy.testing.assert_allclose or equality) so channel order and values are validated; reference symbols: test_load_two_channel, rg_image, spy.Tensor.load_from_image, result, expected.
🧹 Nitpick comments (1)
slangpy/tests/slangpy_tests/test_torchintegration.py (1)
798-804: Rename this test to match what it currently validates.The function name says
read_signature, but the docstring explicitly says this path is not covered yet. A smoke-oriented name would prevent coverage confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 798 - 804, Rename the test function test_diffpair_read_signature to reflect it only exercises the forward pass (e.g., test_diffpair_forward_pass or test_diffpair_forward_smoke), update the function name declaration accordingly, and keep the existing docstring and body unchanged; locate the test function symbol test_diffpair_read_signature in the file and perform the rename so test discovery and coverage correctly reflect what the test validates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@slangpy/tests/slangpy_tests/test_load_image.py`:
- Around line 27-145: Update missing type annotations: annotate the fixtures and
test function parameters and returns — change image_dir() -> str; rgb_image(),
rgba_image(), grey_image(), rg_image() -> Tuple[str, np.ndarray]; and annotate
all test functions' parameters (e.g. test_load_rgb(device, rgb_image) to
test_load_rgb(device: spy.Device, rgb_image: Tuple[str, np.ndarray])) and
similarly for test_load_rgba, test_load_greyscale_single_channel,
test_load_two_channel, test_load_flip_y, test_load_scale_offset,
test_load_greyscale_flag, test_load_linearize; add necessary imports from typing
(Tuple) and use the spy.Device type for the device parameter if available.
Ensure signatures exactly match symbols: image_dir, rgb_image, rgba_image,
grey_image, rg_image, and all test_* functions.
In `@slangpy/tests/slangpy_tests/test_torchintegration.py`:
- Around line 33-35: Tests create CUDA tensors at collection time causing
failures on CPU-only environments; add a module-level skip so collection is
aborted cleanly when CUDA isn't available. At the top of the test module (where
requires_cuda is defined), set a module-wide marker like pytestmark =
pytest.mark.skipif(not torch.cuda.is_available(), reason="CUDA not available")
or call pytest.skip("CUDA not available", allow_module_level=True) early in the
module; this ensures the parametrize that builds CUDA tensors (the CUDA-related
parametrization in test_torchintegration.py) won't execute during collection.
- Around line 999-1011: Replace the broad Exception assertions with the concrete
exception types raised by the code: change the pytest.raises(Exception,
match="shape") around the call to func(bad) to pytest.raises(ValueError,
match="shape") (this covers the shape-check in slangpy/types/tensor.py), and
change the pytest.raises(Exception, match=r"[Cc][Uu][Dd][Aa]") around
func(cpu_tensor) in test_torch_cpu_tensor_rejected to
pytest.raises(RuntimeError, match=r"[Cc][Uu][Dd][Aa]") (this aligns with the
RuntimeError raised by the torch fallback in
slangpy/torchintegration/bridge_fallback.py); keep the same match strings and
the same `func`/`cpu_tensor` call sites created via
helpers.create_function_from_module.
---
Duplicate comments:
In `@slangpy/tests/slangpy_tests/test_load_image.py`:
- Around line 105-112: The test_load_two_channel currently only checks shape;
update it to assert pixel values match the fixture by using the expected value
from rg_image: after loading with spy.Tensor.load_from_image and converting to
numpy (result), compare result to expected (or expected.astype/result dtype)
using an elementwise comparison (e.g., numpy.testing.assert_allclose or
equality) so channel order and values are validated; reference symbols:
test_load_two_channel, rg_image, spy.Tensor.load_from_image, result, expected.
---
Nitpick comments:
In `@slangpy/tests/slangpy_tests/test_torchintegration.py`:
- Around line 798-804: Rename the test function test_diffpair_read_signature to
reflect it only exercises the forward pass (e.g., test_diffpair_forward_pass or
test_diffpair_forward_smoke), update the function name declaration accordingly,
and keep the existing docstring and body unchanged; locate the test function
symbol test_diffpair_read_signature in the file and perform the rename so test
discovery and coverage correctly reflect what the test validates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee84df3f-8ac3-4ddf-982c-8aa93204bb5e
📒 Files selected for processing (7)
slangpy/tests/slangpy_tests/test_load_image.pyslangpy/tests/slangpy_tests/test_raw_dispatch.pyslangpy/tests/slangpy_tests/test_torch_dtypes.pyslangpy/tests/slangpy_tests/test_torchintegration.pysrc/sgl/utils/slangpy.hsrc/slangpy_ext/utils/slangpy.cppsrc/slangpy_ext/utils/slangpytorchtensor.cpp
🚧 Files skipped from review as they are similar to previous changes (5)
- src/slangpy_ext/utils/slangpy.cpp
- slangpy/tests/slangpy_tests/test_torch_dtypes.py
- src/sgl/utils/slangpy.h
- src/slangpy_ext/utils/slangpytorchtensor.cpp
- slangpy/tests/slangpy_tests/test_raw_dispatch.py
Comment on lines
+27
to
+145
| def image_dir(): | ||
| d = os.path.join(tempfile.gettempdir(), "slangpy_test_images") | ||
| os.makedirs(d, exist_ok=True) | ||
| return d | ||
|
|
||
|
|
||
| def _save_image(path: str, data: np.ndarray, pixel_format: spy.Bitmap.PixelFormat): | ||
| b = spy.Bitmap(data, pixel_format=pixel_format) | ||
| b.write(path) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def rgb_image(image_dir): | ||
| data = np.array( | ||
| [[[0.1, 0.2, 0.3], [0.4, 0.5, 0.6]], [[0.7, 0.8, 0.9], [1.0, 0.0, 0.5]]], dtype=np.float32 | ||
| ) | ||
| path = os.path.join(image_dir, "test_rgb.exr") | ||
| _save_image(path, data, spy.Bitmap.PixelFormat.rgb) | ||
| return path, data | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def rgba_image(image_dir): | ||
| data = np.array( | ||
| [ | ||
| [[0.1, 0.2, 0.3, 1.0], [0.4, 0.5, 0.6, 0.8]], | ||
| [[0.7, 0.8, 0.9, 0.5], [1.0, 0.0, 0.5, 0.2]], | ||
| ], | ||
| dtype=np.float32, | ||
| ) | ||
| path = os.path.join(image_dir, "test_rgba.exr") | ||
| _save_image(path, data, spy.Bitmap.PixelFormat.rgba) | ||
| return path, data | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def grey_image(image_dir): | ||
| data = np.array([[0.1, 0.4], [0.7, 1.0]], dtype=np.float32) | ||
| path = os.path.join(image_dir, "test_grey.exr") | ||
| _save_image(path, data, spy.Bitmap.PixelFormat.y) | ||
| return path, data | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def rg_image(image_dir): | ||
| data = np.array([[[0.1, 0.2], [0.4, 0.5]], [[0.7, 0.8], [1.0, 0.0]]], dtype=np.float32) | ||
| path = os.path.join(image_dir, "test_rg.exr") | ||
| _save_image(path, data, spy.Bitmap.PixelFormat.ya) | ||
| return path, data | ||
|
|
||
|
|
||
| def test_load_rgb(device, rgb_image): | ||
| """Load a 3-channel RGB image via Tensor.load_from_image.""" | ||
| path, expected = rgb_image | ||
| tensor = spy.Tensor.load_from_image(device, path) | ||
| result = tensor.to_numpy() | ||
| assert result.shape == expected.shape | ||
| assert np.allclose(result, expected, atol=1e-5) | ||
|
|
||
|
|
||
| def test_load_rgba(device, rgba_image): | ||
| """Load a 4-channel RGBA image.""" | ||
| path, expected = rgba_image | ||
| tensor = spy.Tensor.load_from_image(device, path) | ||
| result = tensor.to_numpy() | ||
| assert result.shape == expected.shape | ||
| assert np.allclose(result, expected, atol=1e-5) | ||
|
|
||
|
|
||
| def test_load_greyscale_single_channel(device, grey_image): | ||
| """Load a single-channel image.""" | ||
| path, expected = grey_image | ||
| tensor = spy.Tensor.load_from_image(device, path) | ||
| result = tensor.to_numpy() | ||
| assert result.shape == expected.shape | ||
| assert np.allclose(result, expected, atol=1e-5) | ||
|
|
||
|
|
||
| def test_load_two_channel(device, rg_image): | ||
| """Load a 2-channel image and verify it produces float2 tensor.""" | ||
| path, expected = rg_image | ||
| tensor = spy.Tensor.load_from_image(device, path) | ||
| result = tensor.to_numpy() | ||
| assert result.ndim == 3 | ||
| assert result.shape[2] == 2 | ||
|
|
||
|
|
||
| def test_load_flip_y(device, rgb_image): | ||
| """Load with flip_y=True — rows should be vertically reversed.""" | ||
| path, expected = rgb_image | ||
| tensor = spy.Tensor.load_from_image(device, path, flip_y=True) | ||
| result = tensor.to_numpy() | ||
| assert result.shape == expected.shape | ||
| assert np.allclose(result, np.flipud(expected), atol=1e-5) | ||
|
|
||
|
|
||
| def test_load_scale_offset(device, rgb_image): | ||
| """Load with scale and offset applied.""" | ||
| path, expected = rgb_image | ||
| tensor = spy.Tensor.load_from_image(device, path, scale=2.0, offset=0.5) | ||
| result = tensor.to_numpy() | ||
| assert np.allclose(result, expected * 2.0 + 0.5, atol=1e-5) | ||
|
|
||
|
|
||
| def test_load_greyscale_flag(device, rgb_image): | ||
| """Load an RGB image with grayscale=True to force single-channel output.""" | ||
| path, _ = rgb_image | ||
| tensor = spy.Tensor.load_from_image(device, path, grayscale=True) | ||
| result = tensor.to_numpy() | ||
| assert result.ndim == 2 | ||
|
|
||
|
|
||
| def test_load_linearize(device, rgb_image): | ||
| """Load with linearize=True to disable sRGB gamma correction.""" | ||
| path, _ = rgb_image | ||
| tensor = spy.Tensor.load_from_image(device, path, linearize=True) | ||
| result = tensor.to_numpy() | ||
| assert result.shape[0] == 2 and result.shape[1] == 2 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: list functions with unannotated args in this file.
python - <<'PY'
import ast
from pathlib import Path
p = Path("slangpy/tests/slangpy_tests/test_load_image.py")
tree = ast.parse(p.read_text(), filename=str(p))
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
missing = [a.arg for a in node.args.args if a.arg not in {"self", "cls"} and a.annotation is None]
if missing:
print(f"Line {node.lineno}: {node.name} -> missing annotations for {missing}")
PYRepository: shader-slang/slangpy
Length of output: 961
Add missing argument type annotations in this test module.
Several functions accept untyped arguments (fixtures and test parameters), violating the repo-wide Python typing requirement.
Example patch pattern (apply consistently)
-@pytest.fixture(scope="module")
-def image_dir():
+@pytest.fixture(scope="module")
+def image_dir() -> str:
d = os.path.join(tempfile.gettempdir(), "slangpy_test_images")
os.makedirs(d, exist_ok=True)
return d
`@pytest.fixture`(scope="module")
-def rgb_image(image_dir):
+def rgb_image(image_dir: str) -> tuple[str, np.ndarray]:
data = np.array(
[[[0.1, 0.2, 0.3], [0.4, 0.5, 0.6]], [[0.7, 0.8, 0.9], [1.0, 0.0, 0.5]]], dtype=np.float32
)
path = os.path.join(image_dir, "test_rgb.exr")
_save_image(path, data, spy.Bitmap.PixelFormat.rgb)
return path, data
-def test_load_rgb(device, rgb_image):
+def test_load_rgb(device: spy.Device, rgb_image: tuple[str, np.ndarray]) -> None:All fixtures and test functions (lines 27–145) require parameter type annotations.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 107-107: Unpacked variable expected is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@slangpy/tests/slangpy_tests/test_load_image.py` around lines 27 - 145, Update
missing type annotations: annotate the fixtures and test function parameters and
returns — change image_dir() -> str; rgb_image(), rgba_image(), grey_image(),
rg_image() -> Tuple[str, np.ndarray]; and annotate all test functions'
parameters (e.g. test_load_rgb(device, rgb_image) to test_load_rgb(device:
spy.Device, rgb_image: Tuple[str, np.ndarray])) and similarly for
test_load_rgba, test_load_greyscale_single_channel, test_load_two_channel,
test_load_flip_y, test_load_scale_offset, test_load_greyscale_flag,
test_load_linearize; add necessary imports from typing (Tuple) and use the
spy.Device type for the device parameter if available. Ensure signatures exactly
match symbols: image_dir, rgb_image, rgba_image, grey_image, rg_image, and all
test_* functions.
Comment on lines
+999
to
+1011
| with pytest.raises(Exception, match="shape"): | ||
| func(bad) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("device_type", DEVICE_TYPES) | ||
| def test_torch_cpu_tensor_rejected(device_type: DeviceType): | ||
| """Non-CUDA torch tensors are rejected by write_shader_cursor_pre_dispatch.""" | ||
| device = helpers.get_torch_device(device_type) | ||
| func = helpers.create_function_from_module(device, "identity", IDENTITY_SRC) | ||
|
|
||
| cpu_tensor = torch.tensor([1.0, 2.0], dtype=torch.float32) | ||
| with pytest.raises(Exception, match=r"[Cc][Uu][Dd][Aa]"): | ||
| func(cpu_tensor) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate broad exception assertions in this test file.
rg -n 'pytest\.raises\(\s*Exception' slangpy/tests/slangpy_tests/test_torchintegration.pyRepository: shader-slang/slangpy
Length of output: 184
🏁 Script executed:
# Read the test functions around the flagged lines for context
cat -n slangpy/tests/slangpy_tests/test_torchintegration.py | sed -n '980,1020p'Repository: shader-slang/slangpy
Length of output: 1512
🏁 Script executed:
# Search for other pytest.raises patterns in this file to see what exception types are used elsewhere
rg -n 'pytest\.raises\(' slangpy/tests/slangpy_tests/test_torchintegration.py | head -20Repository: shader-slang/slangpy
Length of output: 662
🏁 Script executed:
# Check for SlangPyError usage and imports in the test file
rg -n 'SlangPyError|from.*import|import' slangpy/tests/slangpy_tests/test_torchintegration.py | head -30Repository: shader-slang/slangpy
Length of output: 1123
🏁 Script executed:
# Find where validate_tensor_shape is defined and what exception it raises
rg -n 'def validate_tensor_shape' --type pyRepository: shader-slang/slangpy
Length of output: 46
🏁 Script executed:
# Find where write_shader_cursor_pre_dispatch is defined and what exception it raises
rg -n 'def write_shader_cursor_pre_dispatch' --type pyRepository: shader-slang/slangpy
Length of output: 46
🏁 Script executed:
# Search for the exception raise statements in torch-related code
rg -n 'raise.*shape|raise.*[Cc][Uu][Dd][Aa]' slangpy/ --type py | head -20Repository: shader-slang/slangpy
Length of output: 2373
🏁 Script executed:
# Search for shape validation errors related to torch tensors
rg -n 'does not match.*shape|shape.*does not match' --type py -B2 -A2Repository: shader-slang/slangpy
Length of output: 3521
🏁 Script executed:
# Look for CUDA validation in torch tensor handling
rg -n 'CUDA|cuda.*device|device.*cuda' slangpy/torchintegration/ --type py -B2 -A2 | head -40Repository: shader-slang/slangpy
Length of output: 3286
🏁 Script executed:
# Check what func() in the tests actually calls - search for create_function_from_module
rg -n 'def create_function_from_module' --type py -A10Repository: shader-slang/slangpy
Length of output: 689
🏁 Script executed:
# Find where write_shader_cursor_pre_dispatch is implemented (might be in C++ binding)
rg -n 'write_shader_cursor_pre_dispatch' slangpy/torchintegration/ --type py -B5 -A5Repository: shader-slang/slangpy
Length of output: 989
🏁 Script executed:
# Check what exception is raised when func() is called with a CPU tensor
rg -n 'device_type.*1.*if.*cuda|is_cuda.*raise|Tensor must be on CUDA' slangpy/ --type py -B3 -A3Repository: shader-slang/slangpy
Length of output: 1894
🏁 Script executed:
# Verify what tensor.py:304 actually raises (we saw ValueError earlier)
cat -n slangpy/types/tensor.py | sed -n '300,310p'Repository: shader-slang/slangpy
Length of output: 665
Use specific exception types instead of broad Exception in negative-path assertions.
Lines 999 and 1010 use pytest.raises(Exception, ...), which masks the actual exception types being raised and weakens error-contract validation. The underlying code raises concrete exceptions:
- Line 999:
slangpy/types/tensor.pyraisesValueErrorfor shape mismatches - Line 1010:
slangpy/torchintegration/bridge_fallback.pyraisesRuntimeErrorfor non-CUDA tensors
Update assertions to match the actual exception types: ValueError on line 999 and RuntimeError on line 1010. This aligns with the pattern used elsewhere in the file (line 170 correctly asserts ValueError).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@slangpy/tests/slangpy_tests/test_torchintegration.py` around lines 999 -
1011, Replace the broad Exception assertions with the concrete exception types
raised by the code: change the pytest.raises(Exception, match="shape") around
the call to func(bad) to pytest.raises(ValueError, match="shape") (this covers
the shape-check in slangpy/types/tensor.py), and change the
pytest.raises(Exception, match=r"[Cc][Uu][Dd][Aa]") around func(cpu_tensor) in
test_torch_cpu_tensor_rejected to pytest.raises(RuntimeError,
match=r"[Cc][Uu][Dd][Aa]") (this aligns with the RuntimeError raised by the
torch fallback in slangpy/torchintegration/bridge_fallback.py); keep the same
match strings and the same `func`/`cpu_tensor` call sites created via
helpers.create_function_from_module.
Made-with: Cursor # Conflicts: # src/slangpy_ext/utils/slangpytorchtensor.cpp
Made-with: Cursor
Contributor
Author
|
Discussed with Chris this morning; let's back out all of the driver changes, including the stuff from #934 which probably needs a different approach. We'll disable any tests that don't make sense given known issues. |
Back out C++ interop buffer changes (slangpytorchtensor.cpp, slangpy.cpp, slangpy.h) per discussion - these need a different approach in shader-slang#934. Re-restrict test_diffpair_get_shape_grad_only to CUDA only since the interop race condition (shader-slang#929) is no longer fixed in this branch. Also fixes: - test_torch_dtypes.py: guard torch import for macOS - Non-ASCII em dashes in test docstrings Made-with: Cursor
Made-with: Cursor
…ng#937) The contiguity fix for load_buffer_data_from_image is now in PR shader-slang#937. Mark test_load_flip_y as xfail until that fix lands. Made-with: Cursor
…shader-slang#582) Coverage gains were entirely in builtin/ndbuffer.py and types/buffer.py, both of which are being replaced by Tensor. Made-with: Cursor
Made-with: Cursor
These three grad validation tests crash with a nanobind uninitialized instance error during the dispatch pipeline. Skip instead of xfail to avoid potential state contamination from partially constructed C++ objects. Made-with: Cursor
Even the CUDA-only + fallback bridge mode variant triggers the interop buffer lifetime race. Convert from @requires_cuda to @pytest.mark.skip to prevent worker crashes and CUDA context poisoning. Issue shader-slang#929 updated with re-enablement instructions. Made-with: Cursor
Section 1 - Fix vacuous/weak tests: - test_shape: add element_count assertion, fix negative indexing, test copy independence - test_torchintegration: add grad/output assertions to null_grad tests - test_value_torch_pipeline: verify matrix values, packed arg repr Section 2 - Add issue references to all skipped/xfailed tests: - shader-slang#938: TensorMarshall nanobind lifecycle (grad validation crashes) - shader-slang#939: DiffTensor/PrimalTensor sample not implemented - shader-slang#940: Slang compiler bugs (generic vector crash, polynomial derivatives) Section 3 - Pytest best practices: - Remove allow_module_level=True from inside test functions (test_buffer_views) - Fix mutable default arguments in load_test_module (test_raw_dispatch) - Replace bare return with pytest.skip for 3D texture arrays (test_textures) - Move global PYTHON_TYPES mutation to fixture with teardown (test_return_types) Made-with: Cursor
- test_torchintegration: remove "Should this work??" backward call on non-differentiable RWTensor output (shader-slang#574). Other tests already cover backward passes with proper differentiable outputs. - test_textures: remove dead transpose code commented out in PR shader-slang#48 when dimension ordering was fixed upstream. - test_tensor: add assert to test_existential_type_bug compile-only regression test for clarity. Made-with: Cursor
The C++ Shape class has element_count but it's not bound via nanobind. Verify element count indirectly through contiguous strides instead. Made-with: Cursor
The C++ Shape::element_count() method exists but was never registered via nanobind. Skip the test with an explanation rather than removing it, so it can be re-enabled once the binding is added. Made-with: Cursor
ccummingsNV
approved these changes
Apr 30, 2026
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
2 participants
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.
Improve overall test coverage from 73.8% to 82.8% (+9.0 pp), focusing on
torch integration paths on both the Python and C++ sides.
Python coverage: 78.7% → 82.9% (+4.2 pp, 264 newly covered lines)
C++ coverage: 69.2% → 82.7% (+13.5 pp, 1050 newly covered lines)
New and extended test files cover:
Bug fixes included:
(np.flipud result rejected by copy_from_numpy)
Bugs found and filed as separate issues:
preventing texture/sampler descriptor handles (DescriptorMarshall functional API broken: hardcoded StructuredBuffer type prevents texture/sampler descriptor handles #922)
optimization in PR Eliminate heap allocations from cached dispatch hot path #872 (NativeTorchTensorDiffPair::read_signature bypassed by SignatureBuffer optimization #923)
All tests avoid direct Native object construction where possible, using
Python-level APIs (diff_pair(), pack(), backward(), func.map()) to
exercise the underlying C++ code paths naturally.
Fixes #782
Fixes #783
Made with Cursor
Summary by CodeRabbit
Bug Fixes
Tests
Chores