fix(exp): preserve real dtype for real-typed CytnxTensor inputs#13
Open
ultimatile wants to merge 1 commit into
Open
fix(exp): preserve real dtype for real-typed CytnxTensor inputs#13ultimatile wants to merge 1 commit into
ultimatile wants to merge 1 commit into
Conversation
cytnx::linalg::ExpM and ExpH route through eigendecomposition and always return a complex tensor (Eig promotes Float -> ComplexFloat, Double -> ComplexDouble). For real-typed user input the complex backend leaked into the user's tensor, so tci::get_elem<T> read the storage as the requested real type and returned alternating real/imag scalars (the observed "matrix of zeros / ~1e-18"). The anti-Hermitian path additionally crashed for cytnx_float because Cytnx Storage_base::astype rejects complex-to-real casts. Add a unified post-block to both tci::exp overloads that projects the complex result back via Tensor::real() when the user requested a real dtype, then aligns precision via astype as needed. Remove the now-redundant inline astype in the is_float branch. Add in-place regression tests for cytnx_double (general path) and cytnx_float (anti-Hermitian path; checks no-throw + dtype since upstream single-precision Eigh values are wrong); the existing tcict conformance tests only exercise the out-of-place form. Bump external/tcict to def4d3d to pick up the TCICT_SKIP_EXP_SINGLE_PRECISION macro, defined in conformance_runner.cpp so the upstream-blocked single-precision exp tests skip until Cytnx fixes single-precision Eig/Eigh. Closes #8
There was a problem hiding this comment.
Pull request overview
Fixes tci::exp for real-typed CytnxTensor inputs by ensuring the in-place and out-of-place implementations don’t leave a complex-typed Cytnx backend behind (which previously caused incorrect tci::get_elem reads and could crash on cytnx_float due to unsupported complex-to-real astype).
Changes:
- In both
tci::expoverloads, project complex results back to real when the original input dtype is real (Tensor::real()), then align dtype viaastype(original_dtype)before reshaping back. - Add in-place regression tests covering real dtype preservation for
cytnx_double(value checks) andcytnx_float(no-throw + dtype check for the anti-Hermitian path). - Skip tcict single-precision
expconformance cases viaTCICT_SKIP_EXP_SINGLE_PRECISION(pending upstream Cytnx single-precision Eig/Eigh correctness).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| include/tci/cytnx_typed_tensor_impl.h | Ensures exp outputs preserve original real dtype by taking .real() on complex results (when input dtype is real) and then astype-aligning to the original dtype. |
| test/source/test_linear_algebra.cpp | Adds targeted in-place regression tests for real dtype preservation (double) and the float anti-Hermitian crash regression (no-throw + dtype). |
| test/source/conformance_runner.cpp | Defines TCICT_SKIP_EXP_SINGLE_PRECISION to skip known-broken single-precision exp conformance coverage until upstream Cytnx fixes Eig/Eigh. |
| external/tcict | Updates tcict submodule to a revision that recognizes the new skip macro. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Summary
tci::exppreviously left a complex backend in the user's tensor whenever the input was real-typed.tci::get_elemthen read the storage as the requested realelem_t, returning alternating real/imag scalars (the observed "matrix of zeros / ~1e-18" outputs).The
cytnx_floatanti-Hermitian path additionally crashed because Cytnx rejects complex-to-realastype([ERROR] not support type with dtype=4).Both
tci::expoverloads now project the complex eigendecomposition result back to the user's real dtype before reshape.exp(M)of a real matrix is mathematically real, so takingTensor::real()discards only the numerical roundoff residual.Closes #8
Changes
include/tci/cytnx_typed_tensor_impl.h: add a unified post-block to bothtci::expoverloads that callsTensor::real()when the result is complex butoriginal_dtypeis real, then aligns precision viaastype.Remove the now-redundant inline
astypein theis_floatbranch.test/source/test_linear_algebra.cpp: in-place regression tests forcytnx_double(general / diagonal path) andcytnx_float(anti-Hermitian path; checks no-throw + dtype, since upstream single-precisionEighvalues are wrong).The existing tcict conformance tests only exercise the out-of-place form.
external/tcict: bump todef4d3dfor the newTCICT_SKIP_EXP_SINGLE_PRECISIONmacro.test/source/conformance_runner.cpp: defineTCICT_SKIP_EXP_SINGLE_PRECISIONso the upstream-blocked single-precisionexptests skip until Cytnx fixes single-precisionEig/Eigh.Test plan
tcictconformance suite (*test_exp*): 32/32 pass.Both
cytnx_doubleandcytnx_complex128exp tests green; single-precision instances skipped via the new macro.svd,eig,eigvals,trace_partial,io_operations,to_cplx; unrelated totci::exp), 1 skipped (GPU).Net change vs baseline (41 → 30): no new regressions; the 11
tci::expfailures all resolved (5 fixed, 6 skipped via macro).Discovery contract status
cfloatanti-Hermitian via Cytnx single-precisionEigh: scope expanded during implementation to all single-precision (float,cfloat)Eig/Eighpaths intci::exp(6 cases total).All 6 surface the same upstream symptom (
Eig/Eighreturns ~zero eigenvalues for well-conditioned single-precision input).Skipped via
TCICT_SKIP_EXP_SINGLE_PRECISION; will file a separate issue with a minimal Cytnx-only reproducer.tci::eigvals/tci::eigdtype mismatch class: same producer/consumer pattern (cplx_ten_t<CytnxTensor<float>>→complex64, but the implementation forcesComplexDouble).Out of scope for this PR; will track as a separate issue.
Notes
Plan and derivations: #8 (comment)