fix(tests): match blocks by qindices in apply() pybind test helper#985
fix(tests): match blocks by qindices in apply() pybind test helper#985yingjerkao wants to merge 1 commit into
Conversation
The two BlockFermionicUniTensor apply()/apply_() signflip tests failed because _are_nearly_eq compared get_blocks_() by list position, but block order is not canonical: permute().contiguous() keeps the source tensor's block enumeration order, while a freshly constructed reference enumerates blocks in canonical qnum order. The backend apply() output is element-wise identical to the C++ LinAlgElementwise expectation; only the comparison was wrong. Match blocks by get_qindices() and require signflip() flags to agree before comparing raw block data, mirroring the C++ AreNearlyEqUniTensor (tests/test_tools.cpp) while staying sensitive to whether apply() actually folded the pending signs in. Also use the public Bond redirect_() instead of the c_redirect_ shadow binding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the _are_nearly_eq helper function in tests/test_apply_pybind.py to match block UniTensors by their quantum-number indices instead of their positions, and to verify that pending signflips agree. Additionally, it replaces the call to c_redirect_() with redirect_() in _make_bfut3(). There are no review comments, so no further feedback is provided.
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 #985 +/- ##
=======================================
Coverage 30.87% 30.87%
=======================================
Files 229 229
Lines 34757 34757
Branches 14409 14409
=======================================
Hits 10730 10730
Misses 16720 16720
Partials 7307 7307
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:
|
Code ReviewRewrites Correct as far as it goes
🟠 Finding 1 — the fix isn't actually enforced by CICI runs 🟠 Finding 2 — duplicates #1001#1001 (still open, RecommendationGood, correct fix — but please add the Posted by Claude Code on behalf of @pcchen |
Summary
Fixes the two pre-existing failures in
tests/test_apply_pybind.py:TestApplyBlockFermionicUniTensor::test_apply_applies_signflipsTestApplyBlockFermionicUniTensor::test_apply_inplace_applies_signflipsThe backend is correct — the test helper was the bug.
T.apply()output was verified element-wise against the hand-builtBFUT3PERMreference over the full tensor shape: 0 mismatches, including the expected sign-flipped entries (−5, −6, −7, −8), matching the C++LinAlgElementwiseexpectation intests/BlockFermionicUniTensor_test.cpp.Root cause
_are_nearly_eqzippedget_blocks_()by list position, but block order is not canonical:permute().contiguous()keeps the source tensor's block enumeration order, while a freshly constructed reference tensor enumerates blocks in canonical qnum order. Positional pairing therefore compared unrelated blocks. The C++ helperAreNearlyEqUniTensor(tests/test_tools.cpp) explicitly matches blocks by quantum numbers ("they might be not in the same order!"), which is why the C++ test passes.Changes
_are_nearly_eqto match blocks byget_qindices()and requiresignflip()flags to agree before comparing raw block data. Deliberately no sign-folding, so the tests stay sensitive to whetherapply()actually baked the pending signs in (verified: unappliedTvs reference →False, applied →True).Bond.redirect_()instead of thec_redirect_shadow binding (equivalent on master via theBond_conti.pyglue wrapper; avoids a conflict with the in-progress shadow-API removal branch).Test plan
tests/test_apply_pybind.py: 9 passed (was 7 passed / 2 failed)pytests/suite: 28 passed (matches baseline)🤖 Generated with Claude Code