Skip to content

fix: copy optional tensor metadata - #881

Merged
voltjia merged 1 commit into
masterfrom
fix/copy-optional-tensor-metadata
Aug 5, 2026
Merged

fix: copy optional tensor metadata#881
voltjia merged 1 commit into
masterfrom
fix/copy-optional-tensor-metadata

Conversation

@voltjia

@voltjia voltjia commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Copy optional tensor shapes into owning Tensor::Shape members instead of mixing borrowed ShapeView values with owning empty shapes.
  • Copy optional tensor strides into owning Tensor::Strides members across BinaryCrossEntropy, MultiMarginLoss, and both FlashAttention wrappers.

Motivation

InfiniTensor/InfiniRT#38 changed lvalue TensorView::shape() and strides() to return borrowed metadata views. Conditional initializers that paired those views with owning empty metadata no longer had a common C++ type, so InfiniOps failed to compile against the current InfiniRT master.

The smoke-build failure was discovered by #878. A full build found the same compatibility issue in three additional consumers, which this PR fixes together.

Related:

No issue is closed by this PR.

Type of Change

  • feat - new feature / new operator / new platform
  • fix - bug fix
  • perf - performance improvement (no behavioral change)
  • refactor - code restructuring without behavior change
  • test - adding or fixing tests only
  • docs - documentation only
  • build / ci - build system or CI configuration
  • chore - tooling, formatting, or other non-code changes
  • Breaking change (requires a ! in the Conventional Commits prefix or a BREAKING CHANGE: footer)

Platforms Affected

  • CPU (WITH_CPU)
  • NVIDIA (WITH_NVIDIA)
  • Iluvatar (WITH_ILUVATAR)
  • MetaX (WITH_METAX)
  • Cambricon (WITH_CAMBRICON)
  • Moore (WITH_MOORE)
  • Ascend (WITH_ASCEND)
  • PyTorch C++ bindings (WITH_TORCH)
  • Build system / CMake / CI
  • Python bindings / user-facing API

Smoke Test Result

Environment:
  image: accelerator-dev/nvidia:latest
  GPU: NVIDIA A100, CUDA_VISIBLE_DEVICES=5
  InfiniRT: a38977e03f9fafd803c3a4bd0d5810c57bc5a46e
  InfiniOps: 05df7232c7864fe870353343e9d9a270e1e36a1e

$ clang-format --version
clang-format version 21.1.8

$ clang-format --dry-run --Werror <four changed headers>
FORMAT_OK

$ python3 -m pip install '.[dev]' --no-build-isolation --config-settings=cmake.define.INFINI_OPS_SMOKE_BUILD=ON --config-settings=cmake.define.INFINI_RT_ROOT=/work/infinirt-prefix
Successfully built InfiniOps
Successfully installed InfiniOps-0.1.0

$ python3 -m pytest tests/ -m smoke -n 8 -q --tb=short
114 passed, 23 skipped in 26.95s

$ python3 -m pytest tests/test_binary_cross_entropy.py tests/test_multi_margin_loss.py tests/test_flash_attn_varlen_func.py tests/test_flash_attn_with_kvcache.py -q --tb=short -rs
65 passed, 1 skipped, 8 warnings in 2.44s

Test Results on Supported Platforms

Platform Affected Build / Smoke Result Full Result / Notes
CPU Yes smoke passed Included in full build and affected-operator tests
NVIDIA Yes smoke passed Full build passed; affected-operator tests passed
Iluvatar No N/A - not affected N/A - not affected
MetaX No N/A - not affected N/A - not affected
Cambricon No N/A - not affected N/A - not affected
Moore No N/A - not affected N/A - not affected
Ascend No N/A - not affected N/A - not affected

The full CPU+NVIDIA+WITH_TORCH wheel built successfully against the pinned InfiniRT commit. The complete pytest suite ran, but it was not clean: all 50 failures were in unchanged test modules outside the affected operators. The pre-fix baseline cannot reach pytest against this InfiniRT commit because its full build fails on the metadata type errors.

Full build and `pytest` output (trimmed)
$ python3 -m pip install '.[dev]' --no-build-isolation --config-settings=cmake.define.INFINI_RT_ROOT=/work/infinirt-prefix
Created wheel for InfiniOps: filename=infiniops-0.1.0-cp312-cp312-linux_x86_64.whl size=37226025
Successfully built InfiniOps
Successfully installed InfiniOps-0.1.0

$ python3 -m pytest tests/ -n 8 -q --tb=short
50 failed, 13588 passed, 9161 skipped, 127 warnings in 217.93s

Failure groups:
  12  tests/test_tensordot.py
   6  tests/test_moe_sum.py
  32  tests/test_moe_wna16_gemm.py

No failure occurred in:
  tests/test_binary_cross_entropy.py
  tests/test_multi_margin_loss.py
  tests/test_flash_attn_varlen_func.py
  tests/test_flash_attn_with_kvcache.py

Benchmark / Performance Impact

N/A. This is a compile-time compatibility fix. The classes already store owning metadata; the patch makes the required view-to-owner copy explicit in conditional expressions.

Notes for Reviewers

  • The owning copies are intentional: these members retain shape and stride metadata beyond the constructor expression and must not borrow from a temporary tensor view.
  • A repository-wide scan found no other conditional initializer that mixes ShapeView/StridesView with owning empty metadata.
  • Existing tests cover optional tensors both present and absent, so no new behavioral test case was required.

@voltjia
voltjia marked this pull request as ready for review August 5, 2026 07:09
@voltjia
voltjia requested a review from a team August 5, 2026 07:09
@voltjia
voltjia merged commit 1dc5014 into master Aug 5, 2026
18 of 20 checks passed
@voltjia
voltjia deleted the fix/copy-optional-tensor-metadata branch August 5, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant