Repository navigation
[Tensor] Support shape-axis layouts for vector and matrix tensors - #991
hughperkins wants to merge 121 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
this file seems ok-ish
There was a problem hiding this comment.
this file okish
| return t | ||
|
|
||
|
|
||
| def _rebuild_matrix_tensor( |
There was a problem hiding this comment.
_rebuild_matrix_tensor function ok
| return t # type: ignore[reportReturnType] | ||
|
|
||
|
|
||
| def _rebuild_vector_tensor( |
There was a problem hiding this comment.
_rebuild_vector_tensor function ok
| def element_shape(self) -> typing.Tuple[int, ...]: | ||
| return _element_shape_of(self._impl) | ||
|
|
||
| def __reduce__(self) -> typing.Tuple[typing.Any, typing.Tuple[typing.Any, ...]]: |
There was a problem hiding this comment.
reduce function ok
There was a problem hiding this comment.
this file changs ok
| # work on one backend and raise cryptic errors deep in the other. Users who need backend-specific knobs (e.g. | ||
| # ``offset=`` for field offset indexing, ``order=`` for SoA layouts) should call ``qd.field`` / ``qd.ndarray`` directly | ||
| # — they have explicitly opted out of the unified tensor API. | ||
| # |
There was a problem hiding this comment.
this change ok
| _TENSOR_ACCEPTED_KWARGS = frozenset({"backend", "needs_grad", "layout"}) | ||
|
|
||
|
|
||
| def _validate_kwargs(kwargs, *, factory_name, accepted): |
There was a problem hiding this comment.
changes in _validate_kwargs function ok
| return "".join(chr(ord("i") + axis) for axis in layout) | ||
|
|
||
|
|
||
| def tensor(dtype, shape, *, backend=Backend.NDARRAY, layout=None, **kwargs): |
There was a problem hiding this comment.
tensor function ok
There was a problem hiding this comment.
this file ok (reviwed earelir, before the rename/mege)
| reason="Coverage probes change the kernel AST, preventing offline cache hits after reinit", | ||
| ) | ||
| @test_utils.test() | ||
| def test_compound_layout_offline_cache(backend: qd.Backend, dtype: MatrixType, tmp_path: Path) -> None: |
There was a problem hiding this comment.
test_compound_layout_offline_cache ok 🥵
| ], | ||
| ) | ||
| @test_utils.test() | ||
| def test_compound_layout_fastcache(dtype: MatrixType, tmp_path: Path) -> None: |
There was a problem hiding this comment.
test_compound_layout_fastcache ok
There was a problem hiding this comment.
this file ok 🙌
|
Finally, review ok 🙌 Lets wait for CI green-ish, and recheck Mr Codex review. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e13254c79
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @test_utils.test() | ||
| def test_compound_layout_grad(backend: qd.Backend, dtype: MatrixType, request: pytest.FixtureRequest) -> None: | ||
| """Check that gradients retain the layout and compute correct values across different input and output layouts.""" | ||
| if sys.platform == "darwin" and qd.lang.impl.current_cfg().arch == qd.vulkan and backend == qd.Backend.NDARRAY: |
There was a problem hiding this comment.
this xfail seems ok-ish
|
Ok, so we'll merged once CI green-ish. |
|
@codex review |
Builds on the helper renames in #994 and test organization in #996, both merged into main. This PR targets main, adds compound layouts, fuses NumPy import helpers, and removes obsolete layout-rejection and factory-hint tests.
Vector and matrix tensors now accept shape-axis
layout=permutations through all three factories:qd.tensor,qd.Vector.tensor, andqd.Matrix.tensor. Both FIELD and NDARRAY backends keep component axes innermost and user-facing indexing canonical.For example,
qd.Vector.tensor(3, qd.f32, shape=(4, 5), layout=(1, 0))uses physical storage order(5, 4, 3)while exporting canonical shape(4, 5, 3)._total_canonical_shape()and unify scalar, vector, and matrix NumPy imports in_copy_from_numpy(), sharing validation and synchronization while retaining the existing copy kernels.Validation:
4722e3f3d: 87 CPU/CUDA tests passed across compound layouts and convenience factories. Each general compound-layout test has one parameterization covering all four backend/type pairs. Dtype constants were subsequently inlined into the cases; formatting and lint checks passed.5a3479a9b, general compound-layout tests run on all enabled architectures. The available CPU/CUDA environment passed 179 cases withpython tests/run_tests.py -v -t 2 test_tensor_compound_layout test_tensor_factory_vec_mat. NumPy zero-copy remains CPU-only; Torch/DLPack excludes Vulkan and skips Metal FIELD sharing on unsupported PyTorch versions. Metal, AMDGPU, and Vulkan were unavailable in this validation environment.abc6cf109, including the updated factory-hint tests. The unchanged 16 tests also passed independently on the [Tensor] Move test_tensor_pr560_fixes.py tests into other test files #996 base._total_physical_layout()extraction atbfa0e9148: 227 CPU/CUDA compound-layout and interop cases passed, with 2 skips. The subsequent variable rename and merge ofmainpassed formatting, lint, and non-ASCII checks; the merge leaves branch file contents unchanged.ed7b8ffda: 589 CPU/CUDA cases passed across ndarray, factories, compound layouts, error handling, gradients, wrapper surface, and layouts. The subsequent base-branch ASCII punctuation fix leaves this branch's file contents unchanged.22c0cceca: 412 CPU/CUDA ndarray and layout cases passed after unifying NumPy imports. The subsequent_copy_to_torch()rename passed formatting and lint checks.3062f762c: 122 compound-layout CPU/CUDA cases passed after adding_total_canonical_shape(), including copied and zero-copy Torch interop.9d5be3558: C++ rebuild succeeded; 412 ndarray and layout CPU/CUDA cases passed after the physical-shape rename.b09c32bd577085dd5ad9249680e3d6083d510b59: 383 CPU/CUDA cases passed after removing the private factory helpers. These cover factories, compound layouts, error messages, gradients, wrappers, and layouts.eeb56dca6282854e2fdef67783b3ba1b897e2be4also passed 589 existing regression cases, with 2 skips.Reproduce the shape-helper validation with:
Reproduce the factory validation with:
The earlier regression run also exercised:
Fixes #936.