-
Notifications
You must be signed in to change notification settings - Fork 62
ToT: conj() on nested-tile contractions; real-plain x complex-ToT products via one real gemm #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2ff7060
59cc2e8
0a8dea7
586adc7
c45aeb3
8187a19
be546dd
ef1ecb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2062,14 +2062,41 @@ class Tensor { | |
| // early exit for empty this | ||
| if (empty()) return *this; | ||
|
|
||
| if constexpr (is_arena_tensor_v<value_type>) { | ||
| // Arena inner cells: route through each cell's own in-place scale_to (the | ||
| // free arena kernel), which handles a ComplexConjugate factor by | ||
| // conjugating each arena scalar in place. Going through `cell *= factor` | ||
| // would instead select the generic operator*=(.., ComplexConjugate) -> | ||
| // detail::conj(cell), which has no value-returning conj for ArenaTensor. | ||
| return inplace_unary( | ||
| [factor](value_type& MADNESS_RESTRICT c) { c.scale_to(factor); }); | ||
| if constexpr (detail::is_complex_conjugate_v<Scalar> && | ||
| !detail::is_complex_v<numeric_type>) { | ||
| // real elements: conj is the identity, only the scale (if any) remains | ||
| if constexpr (std::is_same_v<Scalar, detail::ComplexConjugate<void>>) | ||
| return *this; | ||
| else if constexpr (std::is_same_v<Scalar, detail::ComplexConjugate< | ||
| detail::ComplexNegTag>>) | ||
| return neg_to(); | ||
| else | ||
| return scale_to(factor.factor()); | ||
| } else if constexpr (is_arena_tensor_v<value_type>) { | ||
| // Arena inner cells: the free arena kernel scales -- or, for a | ||
| // ComplexConjugate factor, conjugates and scales -- every arena scalar | ||
| // in place. (The member ArenaTensor::scale_to takes numeric factors | ||
| // only; `cell *= factor` is the same kernel via the ArenaTensor | ||
| // operator*= overloads.) | ||
| return inplace_unary([factor](value_type& MADNESS_RESTRICT c) { | ||
| ::TiledArray::scale_to(c, factor); | ||
| }); | ||
| } else if constexpr (detail::is_complex_conjugate_v<Scalar> && | ||
| detail::is_ta_tensor_v<value_type>) { | ||
|
Comment on lines
+2084
to
+2085
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This reads the opposite way round: the branch fixes the BTAS case rather than leaving it broken. On Why the Taking the actionable half of the suggestion: added |
||
| // Owning nested cells with a ComplexConjugate factor: conjugate (and | ||
| // scale) each cell IN PLACE. `cell *= factor` would resolve to | ||
| // `cell = conj(cell) * S`, i.e. a fresh allocation, copy and free of | ||
| // every cell. | ||
| return inplace_unary([factor](value_type& MADNESS_RESTRICT res) { | ||
| if constexpr (std::is_same_v<Scalar, detail::ComplexConjugate<void>>) | ||
| res.conj_to(); | ||
| else if constexpr (std::is_same_v<Scalar, detail::ComplexConjugate< | ||
| detail::ComplexNegTag>>) { | ||
| res.conj_to(); | ||
| res.neg_to(); | ||
| } else | ||
| res.conj_to(factor.factor()); | ||
| }); | ||
| } else { | ||
| return inplace_unary( | ||
| [factor](value_type& MADNESS_RESTRICT res) { res *= factor; }); | ||
|
|
@@ -3387,6 +3414,14 @@ class Tensor { | |
| Tensor& gemm(const Tensor<As...>& A, const Tensor<Bs...>& B, const W alpha, | ||
| const math::GemmHelper& gemm_helper) { | ||
| numeric_type beta = 1; | ||
| // A ComplexConjugate<...> factor is not a gemm alpha: conj does not | ||
| // distribute into the sum of products, so the expression layer applies it | ||
| // to the finished result (ContractReduce's finalization; see | ||
| // ContEngine::outer_factor) and hands the gemm a numeric alpha. | ||
| static_assert(!detail::is_complex_conjugate_v<W>, | ||
| "Tensor::gemm: a ComplexConjugate<...> alpha cannot be " | ||
| "applied inside the gemm; conjugate the finished result"); | ||
| const numeric_type alpha_n = static_cast<numeric_type>(alpha); | ||
| if (this->empty()) { | ||
| *this = | ||
| Tensor(gemm_helper.make_result_range<range_type>(A.range_, B.range()), | ||
|
|
@@ -3411,7 +3446,7 @@ class Tensor { | |
| } | ||
| for (size_t i = 0; i < this->nbatch(); ++i) { | ||
| auto Ci = this->batch(i); | ||
| TiledArray::gemm(alpha, A.batch(i), B.batch(i), | ||
| TiledArray::gemm(alpha_n, A.batch(i), B.batch(i), | ||
| twostep ? numeric_type(0) : numeric_type(1), Ci, | ||
| gemm_helper); | ||
| } | ||
|
|
@@ -3465,7 +3500,7 @@ class Tensor { | |
| #else // TA_ENABLE_TILE_OPS_LOGGING | ||
| for (size_t i = 0; i < this->nbatch(); ++i) { | ||
| auto Ci = this->batch(i); | ||
| TiledArray::detail::gemm(alpha, A.batch(i), B.batch(i), beta, Ci, | ||
| TiledArray::detail::gemm(alpha_n, A.batch(i), B.batch(i), beta, Ci, | ||
| gemm_helper); | ||
| } | ||
| #endif // TA_ENABLE_TILE_OPS_LOGGING | ||
|
|
@@ -3572,7 +3607,21 @@ class Tensor { | |
| if constexpr (detail::is_numeric_v<V> && is_tensor_view_v<U> && | ||
| is_tensor_view_v<value_type>) { | ||
| using Real = std::remove_cv_t<typename value_type::value_type>; | ||
| if constexpr (std::is_same_v<std::remove_cv_t<V>, Real>) { | ||
| using Vr = std::remove_cv_t<V>; | ||
| // Both slabs are viewed through Vr* below, so the left ToT's inner | ||
| // scalar must be the result's. Same element type: one gemm in that | ||
| // type. Real plain scalars (Vr) against complex<Vr> inner cells: the | ||
| // complex slabs are viewed as real matrices with the inner extent | ||
| // doubled (re,im interleaved), so one real gemm with alpha = beta = 1 | ||
| // accumulates both parts exactly (detail::is_interleaved_real_view_v | ||
| // carries the layout terms; a mismatch falls back to the per-cell loop). | ||
| constexpr bool left_matches = | ||
| std::is_same_v<std::remove_cv_t<typename U::value_type>, Real>; | ||
| constexpr bool same_type = left_matches && std::is_same_v<Vr, Real>; | ||
| constexpr bool interleaved = left_matches && !std::is_same_v<Vr, Real> && | ||
| detail::is_interleaved_real_view_v<Real, Vr>; | ||
| constexpr integer cw = interleaved ? 2 : 1; // reals per inner element | ||
| if constexpr (same_type || interleaved) { | ||
| // Both operand orientations are served: the plain (right) matrix's | ||
| // transpose is passed to BLAS as its op, and a transposed nested | ||
| // (left) tile only changes which cells form row m (cells (k,m) are | ||
|
|
@@ -3685,6 +3734,9 @@ class Tensor { | |
| if (detail::scale_gemm_timing_enabled()) { | ||
| detail::g_scale[0].gemm_runs.fetch_add( | ||
| 1, std::memory_order_relaxed); | ||
| // element multiply-adds (A in inner elements), the same | ||
| // unit as fb_flop and the same-type path, so the | ||
| // [scale-timing] coverage ratios compare like with like | ||
| detail::g_scale[0].gemm_flop.fetch_add( | ||
| 2ull * static_cast<std::uint64_t>(K) * | ||
| static_cast<std::uint64_t>(N) * | ||
|
|
@@ -3699,10 +3751,12 @@ class Tensor { | |
| right_no_trans ? TiledArray::math::blas::Transpose | ||
| : TiledArray::math::blas::NoTranspose, | ||
| TiledArray::math::blas::NoTranspose, | ||
| /*M=*/N, /*N=*/Ai, /*K=*/K, Real(1), | ||
| /*M=*/N, /*N=*/Ai * cw, /*K=*/K, Vr(1), | ||
| /*A=*/right_data, /*lda=*/right_no_trans ? N : K, | ||
| /*B=*/lc(0).data(), /*ldb=*/ldb, Real(1), | ||
| /*C=*/rc0[0].data(), /*ldc=*/ldc); | ||
| /*B=*/reinterpret_cast<const Vr*>(lc(0).data()), | ||
| /*ldb=*/ldb * cw, Vr(1), | ||
| /*C=*/reinterpret_cast<Vr*>(rc0[0].data()), | ||
| /*ldc=*/ldc * cw); | ||
| } else { // per-cell AXPY fallback for this row | ||
| if (detail::scale_gemm_timing_enabled()) { | ||
| // classify fallback reason (re-scan; observation only, does | ||
|
|
@@ -3773,7 +3827,17 @@ class Tensor { | |
| if constexpr (detail::is_numeric_v<U> && is_tensor_view_v<V> && | ||
| is_tensor_view_v<value_type>) { | ||
| using Real = std::remove_cv_t<typename value_type::value_type>; | ||
| if constexpr (std::is_same_v<std::remove_cv_t<U>, Real>) { | ||
| using Ur = std::remove_cv_t<U>; | ||
| // see the tot_x_t block: the right ToT's inner scalar must be the | ||
| // result's; then same type, or real plain x complex<Ur> inner cells via | ||
| // the re,im-interleaved real gemm | ||
| constexpr bool right_matches = | ||
| std::is_same_v<std::remove_cv_t<typename V::value_type>, Real>; | ||
| constexpr bool same_type = right_matches && std::is_same_v<Ur, Real>; | ||
| constexpr bool interleaved = right_matches && !std::is_same_v<Ur, Real> && | ||
| detail::is_interleaved_real_view_v<Real, Ur>; | ||
| constexpr integer cw = interleaved ? 2 : 1; // reals per inner element | ||
| if constexpr (same_type || interleaved) { | ||
| // Both orientations, as in the tot_x_t block: the plain (left) | ||
| // matrix's transpose goes to BLAS as its op; a transposed nested | ||
| // (right) tile stores cell (k,n) at n*K + k, so column n's cells are | ||
|
|
@@ -3866,6 +3930,7 @@ class Tensor { | |
| if (detail::scale_gemm_timing_enabled()) { | ||
| detail::g_scale[1].gemm_runs.fetch_add( | ||
| 1, std::memory_order_relaxed); | ||
| // element multiply-adds, the unit of fb_flop (see above) | ||
| detail::g_scale[1].gemm_flop.fetch_add( | ||
| 2ull * static_cast<std::uint64_t>(M) * | ||
| static_cast<std::uint64_t>(K) * | ||
|
|
@@ -3880,10 +3945,13 @@ class Tensor { | |
| left_no_trans ? TiledArray::math::blas::NoTranspose | ||
| : TiledArray::math::blas::Transpose, | ||
| TiledArray::math::blas::NoTranspose, | ||
| /*M=*/M, /*N=*/Ai, /*K=*/K, Real(1), | ||
| /*M=*/M, /*N=*/Ai * cw, /*K=*/K, Ur(1), | ||
| /*A=*/left_data, /*lda=*/left_no_trans ? K : M, | ||
| /*B=*/right_data[rcell(0, n)].data(), /*ldb=*/sbc, Real(1), | ||
| /*C=*/this_data[n].data(), /*ldc=*/ldc); | ||
| /*B=*/ | ||
| reinterpret_cast<const Ur*>(right_data[rcell(0, n)].data()), | ||
| /*ldb=*/sbc * cw, Ur(1), | ||
| /*C=*/reinterpret_cast<Ur*>(this_data[n].data()), | ||
| /*ldc=*/ldc * cw); | ||
| } else { // per-cell AXPY fallback for this column | ||
| if (detail::scale_gemm_timing_enabled()) { | ||
| // classify fallback reason (re-scan; observation only) + | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked against the branch head.
c("k,i;b,a") = conj(a("i,j;a") * b("j,k;b"))onTensor<ArenaTensor<complex<double>>>compiles and matches an owning-tile reference exactly (err 0); so do the outer-only and inner-only permuted variants, the scaled permuted form2.0 * conj(...), and permuted unaryconj.It is also already covered by a passing assertion in
tot_conj/arena_conj_of_product— the fourth check there is literallyc("k,i;b,a") = conj(a("i,j;a") * b("j,k;b")).The premise does not hold: the permuted arena finalization does not route cells through a value-returning
Tensor::scale(factor, perm). No change made.