fix(pybind)!: guard UniTensor//UniTensor floor-division (closes #934 hole from #997) - #1049
Merged
Merged
Conversation
…hole from #997) #997 removed elementwise UniTensor/UniTensor division by guarding the `__truediv__` / `__itruediv__` dunders (`/`, `/=`), but Python's floor-division maps to the *distinct* `__floordiv__` / `__ifloordiv__` dunders, which were left routing straight to `linalg::Div` / `Div_`. That left `ut1 // ut2` and `ut1 //= ut2` as a silent back-door for the exact Hadamard quotient `ut1 / ut2` was made to reject -- a basis-dependent op with no tensor-network meaning (#934). The floordiv path had no test coverage in binding_unitensor_elementwise_test.py, so the gap went unnoticed. Fix: the two `UniTensor⊗UniTensor` floordiv overloads now `raise_unitensor_elementwise_removed`, mirroring their truediv siblings and reusing the same `kUniTensorDivRemovedGuidance` message. Scalar floordiv (`ut // 2.0`, `ut //= 2.0`) is retained, matching how scalar `/` is kept. Tests: adds test_floordiv_unitensor_unitensor_raises, test_ifloordiv_unitensor_unitensor_raises, and test_scalar_floordiv_unitensor_still_works (scope boundary). Full suite: 23 passed. BREAKING CHANGE: `ut1 // ut2` and `ut1 //= ut2` now raise TypeError instead of performing elementwise division -- completing the removal announced in #997. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the elementwise floor-division (// and //=) operators between two UniTensor objects, raising a TypeError with guidance instead. This prevents bypassing the restriction on elementwise division (/ and /=) which was previously removed. Scalar floor-division (e.g., ut // 2.0) remains supported and routes to linalg::Div. Corresponding unit tests have been added to verify that the two-UniTensor floor-division raises the expected error and that scalar floor-division continues to work. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1049 +/- ##
==========================================
+ Coverage 57.71% 57.72% +0.01%
==========================================
Files 229 229
Lines 33468 33471 +3
Branches 71 71
==========================================
+ Hits 19315 19321 +6
+ Misses 14132 14129 -3
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
yingjerkao
added a commit
that referenced
this pull request
Jul 14, 2026
Brings in the latest master (98de76f): #1001 (Bond made immutable + BlockFermionicUniTensor signflip encapsulated via reset_signflip_/ erase_signflip_), #1047 (Exp/Expf consolidated into one dtype-preserving Exp; Expf/Expf_ deprecated), #1049 (UniTensor//UniTensor floor-division guarded), and the build-test-workflow / cross-revision-benchmark skills. Conflict resolution: - tests/DenseUniTensor_test.cpp: both sides added a test after combineBond_is_in_place -- this PR's combineBond_is_out_of_place and master's #846 aliasing regression (CombineBondForceDoesNotRewriteUserHeldBond). Kept both. Adaptations required by the merge (master's incoming tests were written against master's in-place combineBond(); this PR redefined bare combineBond() as out-of-place per #421/#422): - tests/DenseUniTensor_test.cpp + tests/BlockUniTensor_test.cpp: the #846 CombineBondForceDoesNotRewriteUserHeldBond tests assert the receiver itself mutates (ut.bonds()[0].dim() == combined), so they now call the in-place combineBond_() spelling instead of bare combineBond() (which now returns a new tensor and leaves the receiver unchanged). The #846 aliasing invariant is unchanged -- the in-place force path still clones the bond entry before force_combineBond_. - pytests/apply_test.py: master's newly-moved file called the deprecated .Norm().item(); migrated to .norm() (a native float in Python) to keep the suite clean under -W error::DeprecationWarning, matching this PR's migration of every other in-tree Norm() caller. Auto-merged cleanly (verified as a proper union, no dropped hunks): the PR's norm()->Scalar + underscore renames (set_name_/set_label_/tag_) coexist with master's Bond-immutable retype()/signflip refactor across the linalg decomposition files (Svd/Gesvd/Qr/Qdr/Rsvd/*_truncate); the floordiv guard (UniTensor//UniTensor raises, scalar // kept) sits with the amended #934 elementwise policy in unitensor_py.cpp; Exp consolidation + norm/Norm bindings coexist in linalg.hpp/linalg_py.cpp. Verification (CI-matching openblas-cpu, non-ASan): - C++ test_main: 1248 tests ran, 1237 passed / 11 skipped / 0 failed. - pybind module compiles clean; full pytests/ suite 185 passed / 1 skipped. - merge-adapted test files pass under -W error::DeprecationWarning. - clang-format v14 clean. Note (pre-existing, out of scope; flagged for a follow-up): under an ASan build, BlockUniTensor::combineBonds (src/BlockUniTensor.cpp:1958,1990) trips memcpy-param-overlap -- it memcpy()s an overlapping intra-vector shift where memmove() is required. This is latent UB unchanged by this merge (identical on master); the forward copy yields correct results, so it is benign on the non-ASan CI build and only ASan flags it. master's #846 test (combining two bonds of a 4-bond tensor, leaving a trailing bond) is the first case to surface it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yingjerkao
added a commit
that referenced
this pull request
Jul 15, 2026
…ue-division (#941) Reimplements the deferred half of #1015 on current master, which had since refactored the same six arithmetic files for rank-zero support (#1026), zero-extent (#1043), and type_promote (#984). - Convert CPU out-of-place AND in-place Add/Sub/Mul/Div/Cpr/Mod to typed std::visit over as_storage_variant() with storage_cast<TO> and typed *InternalImpl<TO,TL,TR> kernels; the dead legacy dispatch tables are removed. - True division (#941): `/` and `/=` yield a floating result (int/int -> Double) via make_floating_point_dtype; the rank-zero / broadcast output still routes through #1026's init_broadcast_binary_output (seeded with the floating dtype). - Mixed-dtype in-place TENSOR ops promote the LHS via storage_as_type_or_replace (no more truncation into the narrower LHS storage -- #941's core bug). - Weak-scalar (#980, per #1015's binding ruling): `tensor op= python-scalar` (a rank-0 RHS via scalar_as_rank0_tensor) PRESERVES the LHS dtype for Add/Sub/Mul and follows true-division for Div (Int64 /= 2.0 -> Double, Float /= 3.0 -> Float), keyed on Rt.rank()==0 in DispatchInplaceArithmeticCPU; complex-into-real still throws. - floordiv conforms to #1049 (scalar `ut // 2.0` kept, `ut1 // ut2` raises); #1015's original unbind-all is dropped. - normalize_() now promotes an integer block to Double under true division; its stale "stays Int32" comment is updated. Mod keeps an eager is_complex guard so a zero-extent complex Mod still throws (ZeroExtentLinalgTest). CPU only; GPU stays on the legacy dtype tables behind the unchanged public API (#1013 tracks the GPU conversion). Testing: full openblas-cpu (non-ASan) test_main 1784 passed / 11 skipped / 0 failed -- MixedDtypeArithmetic regressions, weak-scalar Tensor.ScalarInplace*, ZeroExtentLinalgTest, and DenseUniTensorTest true-division/normalize all green; clang-format v14 clean. Reimplements ff30d00, deferred from #1015 (#941). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yingjerkao
added a commit
that referenced
this pull request
Jul 15, 2026
…ue-division (#941) Reimplements the deferred half of #1015 on current master, which had since refactored the same six arithmetic files for rank-zero support (#1026), zero-extent (#1043), and type_promote (#984). - Convert CPU out-of-place AND in-place Add/Sub/Mul/Div/Cpr/Mod to typed std::visit over as_storage_variant() with storage_cast<TO> and typed *InternalImpl<TO,TL,TR> kernels; the dead legacy dispatch tables are removed. - True division (#941): `/` and `/=` yield a floating result (int/int -> Double) via make_floating_point_dtype; the rank-zero / broadcast output still routes through #1026's init_broadcast_binary_output (seeded with the floating dtype). - Mixed-dtype in-place TENSOR ops promote the LHS via storage_as_type_or_replace (no more truncation into the narrower LHS storage -- #941's core bug). - Weak-scalar (#980, per #1015's binding ruling): `tensor op= python-scalar` (a rank-0 RHS via scalar_as_rank0_tensor) PRESERVES the LHS dtype for Add/Sub/Mul and follows true-division for Div (Int64 /= 2.0 -> Double, Float /= 3.0 -> Float), keyed on Rt.rank()==0 in DispatchInplaceArithmeticCPU; complex-into-real still throws. - floordiv conforms to #1049 (scalar `ut // 2.0` kept, `ut1 // ut2` raises); #1015's original unbind-all is dropped. - normalize_() now promotes an integer block to Double under true division; its stale "stays Int32" comment is updated. Mod keeps an eager is_complex guard so a zero-extent complex Mod still throws (ZeroExtentLinalgTest). CPU only; GPU stays on the legacy dtype tables behind the unchanged public API (#1013 tracks the GPU conversion). Testing: full openblas-cpu (non-ASan) test_main 1784 passed / 11 skipped / 0 failed -- MixedDtypeArithmetic regressions, weak-scalar Tensor.ScalarInplace*, ZeroExtentLinalgTest, and DenseUniTensorTest true-division/normalize all green; clang-format v14 clean. Reimplements ff30d00, deferred from #1015 (#941). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yingjerkao
added a commit
that referenced
this pull request
Jul 15, 2026
) Address PR #1056 review (Codex): this PR unbinds Tensor `__floordiv__` / `__rfloordiv__` / `__ifloordiv__` (and the keep-set-generated `c__ifloordiv__`) in pybind/tensor_py.cpp, but the shipped PEP 561 stub cytnx/cytnx/__init__.pyi still advertised those overloads. mypy/Pylance would therefore accept `t // 2` even though it now raises TypeError at runtime. Remove exactly the Tensor floordiv-family entries from the stub, each verified absent from the built module (`__floordiv__`, `__rfloordiv__`, `__ifloordiv__`, `c__ifloordiv__`). This is a scoped surgical edit rather than a full `tools/generate_stubs.py` regeneration: this environment only has Python 3.12 (the committed stubs are generated with the lowest supported 3.10) plus a different numpy, so a wholesale regen emits ~2600 lines of unrelated tooling/version churn. UniTensor scalar floordiv is intentionally kept (#1049) and left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yingjerkao
added a commit
that referenced
this pull request
Jul 21, 2026
Problem: cytnx.Scalar leaked C++'s Scalar value type onto the Python surface. Python code should work with native numbers and derive dtype from a tensor/storage's dtype(), never handle a Scalar object. Fix: - Drop the Scalar binding entirely (delete pybind/scalar_py.cpp, its scalar_binding() decl/call in cytnx.cpp, and its CMakeLists entry). - Remove the redundant accept-Scalar arithmetic/compare overloads from Tensor (tensor_py.cpp) and UniTensor (unitensor_py.cpp). Each has a native double/complex128 sibling plus the numpy_scalar/py::int_ keep set, so plain Python numbers still dispatch unchanged -- including scalar UniTensor '//' and '%', kept per #1049. - Migrate the linalg coefficients that were Scalar (Ger, Gemm, Gemm_, Lanczos_Exp) to native real overloads plus a complex_arg.hpp ComplexArg fallback, matching the existing ExpH/ExpM pattern so a real argument stays real and a Python complex is accepted on 3.10. - Delete the now-obsolete Python Scalar tests (pytests/scalar_variant_test.py and the Scalar-constructor cases in keep_set_dispatch_test.py); the C++ variant behaviour stays covered by tests/Scalar_test.cpp. Update the __ne__/reverse-op tests to use native operands. - Regenerate the committed .pyi stubs (Scalar class removed; Ger/Gemm/ Gemm_/Lanczos_Exp gain real/complex overloads). Testing: pytest pytests/ -> 260 passed, 1 skipped. Smoke-checked that cytnx.Scalar is gone, Tensor/UniTensor scalar arithmetic (incl. ut//2.0), and Ger/Gemm/Gemm_/Lanczos_Exp accept native ints/floats/complex with the expected result dtypes. clang-format v14 clean. Closes #1045. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
yingjerkao
added a commit
that referenced
this pull request
Jul 22, 2026
) Problem: cytnx.Scalar leaked C++'s Scalar value type onto the Python surface. Python code should work with native numbers and derive dtype from a tensor/storage's dtype(), never handle a Scalar object. Fix: - Drop the Scalar binding entirely (delete pybind/scalar_py.cpp, its scalar_binding() decl/call in cytnx.cpp, and its CMakeLists entry). - Remove the redundant accept-Scalar arithmetic/compare overloads from Tensor (tensor_py.cpp) and UniTensor (unitensor_py.cpp). Each has a native double/complex128 sibling plus the numpy_scalar/py::int_ keep set, so plain Python numbers still dispatch unchanged -- including scalar UniTensor '//' and '%', kept per #1049. - Migrate the linalg coefficients that were Scalar (Ger, Gemm, Gemm_, Lanczos_Exp) to native real overloads plus a complex_arg.hpp ComplexArg fallback, matching the existing ExpH/ExpM pattern so a real argument stays real and a Python complex is accepted on 3.10. - Delete the now-obsolete Python Scalar tests (pytests/scalar_variant_test.py and the Scalar-constructor cases in keep_set_dispatch_test.py); the C++ variant behaviour stays covered by tests/Scalar_test.cpp. Update the __ne__/reverse-op tests to use native operands. - Regenerate the committed .pyi stubs (Scalar class removed; Ger/Gemm/ Gemm_/Lanczos_Exp gain real/complex overloads). Testing: pytest pytests/ -> 260 passed, 1 skipped. Smoke-checked that cytnx.Scalar is gone, Tensor/UniTensor scalar arithmetic (incl. ut//2.0), and Ger/Gemm/Gemm_/Lanczos_Exp accept native ints/floats/complex with the expected result dtypes. clang-format v14 clean. Closes #1045. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Problem
#997 removed elementwise
UniTensor ⊘ UniTensordivision from the Python surface — but only for the true-division dunders. Python's/maps to__truediv__, which #997 correctly guarded (raisesTypeError). Python's floor-division//maps to a distinct dunder,__floordiv__, and theUniTensor⊗UniTensoroverloads there were left routing straight tolinalg::Div/Div_:So
//and//=were a silent back-door for the exact basis-dependent Hadamard quotient that/was made to reject (#934 — no well-defined tensor-network meaning, typically produces inf/nan). The floor-division path had no test coverage inbinding_unitensor_elementwise_test.py, which is why the gap slipped through #997.(There is no floor-multiply operator, so the
*removal has no analogous hole — this is division-only.)Fix
The two
UniTensor⊗UniTensorfloordiv overloads nowraise_unitensor_elementwise_removed, mirroring their__truediv__/__itruediv__siblings exactly and reusing the samekUniTensorDivRemovedGuidancemessage:pybind/unitensor_py.cpp__floordiv__(UniTensor, UniTensor)→ raisesTypeError(//)pybind/unitensor_py.cpp__ifloordiv__(UniTensor, UniTensor)→ raisesTypeError(//=)Scalar floordiv is retained (
ut // 2.0,ut //= 2.0), matching how scalar/is kept — only the two-UniTensor overloads change.Testing
pytests/binding_unitensor_elementwise_test.py— added:test_floordiv_unitensor_unitensor_raises—ut1 // ut2raisesTypeErrortest_ifloordiv_unitensor_unitensor_raises—ut1 //= ut2raisesTypeErrortest_scalar_floordiv_unitensor_still_works— pins the scope boundary (scalar//still works)Rebuilt
openblas-cpuand ran the suite: 23 passed (was 20).Breaking change
ut1 // ut2andut1 //= ut2now raiseTypeErrorinstead of performing elementwise division — this completes the removal announced as a BREAKING CHANGE in #997.Closes the floor-division gap in #934 / #997.
🤖 Generated with Claude Code