From 9c7a949bfe6ff7e41459bd2ff13f5cbe35cdea51 Mon Sep 17 00:00:00 2001 From: "Peter Doak (epd)" Date: Fri, 23 Aug 2024 17:10:46 -0400 Subject: [PATCH 1/2] Widening assignment for Matrix is fine and needed for hdf5 output --- src/Containers/OhmmsPETE/OhmmsMatrix.h | 27 ++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/Containers/OhmmsPETE/OhmmsMatrix.h b/src/Containers/OhmmsPETE/OhmmsMatrix.h index 413fffcd78..2958225cdf 100644 --- a/src/Containers/OhmmsPETE/OhmmsMatrix.h +++ b/src/Containers/OhmmsPETE/OhmmsMatrix.h @@ -2,7 +2,7 @@ // This file is distributed under the University of Illinois/NCSA Open Source License. // See LICENSE file in top directory for details. // -// Copyright (c) 2021 QMCPACK developers. +// Copyright (c) 2024 QMCPACK developers. // // File developed by: Ken Esler, kpesler@gmail.com, University of Illinois at Urbana-Champaign // Miguel Morales, moralessilva2@llnl.gov, Lawrence Livermore National Laboratory @@ -35,6 +35,7 @@ class Matrix using size_type = typename Container_t::size_type; using iterator = typename Container_t::iterator; using This_t = Matrix; + using Alloc_t = Alloc; Matrix() : D1(0), D2(0), TotSize(0) {} // Default Constructor initializes to zero. @@ -154,7 +155,7 @@ class Matrix template void assignUpperLeft(const Matrix& from) { - auto& this_ref = *this; + auto& this_ref = *this; const size_type cols = std::min(this_ref.cols(), from.cols()); const size_type rows = std::min(this_ref.rows(), from.rows()); for (int i = 0; i < rows; ++i) @@ -163,14 +164,36 @@ class Matrix } // Assignment Operators + /// From another Matrix with matching type. inline This_t& operator=(const This_t& rhs) { resize(rhs.D1, rhs.D2); + // I don't understand why it would be desirable to have this compile + // but just do the resize and not the assigment, just seems like a surprising foot gun. if (qmc_allocator_traits::is_host_accessible) assign(*this, rhs); return *this; } + /** From a Matrix with a narrower value_type + * so this is a widening assignment and there is no loss of precision. + * Giving it the same semantics as the matching type matrix is desirable. + */ + template= sizeof(typename Matrix::value_type), void>> + This_t& operator=(const Matrix& rhs) + { + static_assert(qmc_allocator_traits::is_host_accessible && + qmc_allocator_traits::Alloc_t>::is_host_accessible); + resize(rhs.size1(), rhs.size2()); + assign(*this, rhs); + return *this; + } + + /** From any type except the above two cases that assign can resolve. + * Historically this allowed narrowing assigments from one Matrix type to another. + * I am not sure this was intentional. + */ template> This_t& operator=(const RHS& rhs) { From 9bb8198fad634f0fa81bda8f0755a40875850db0 Mon Sep 17 00:00:00 2001 From: "Peter Doak (epd)" Date: Fri, 23 Aug 2024 17:24:41 -0400 Subject: [PATCH 2/2] adding testing of "interesting" Matrix assignment behavior to utest --- src/Containers/OhmmsPETE/tests/test_Matrix.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Containers/OhmmsPETE/tests/test_Matrix.cpp b/src/Containers/OhmmsPETE/tests/test_Matrix.cpp index 65b6777f18..0f01bd424b 100644 --- a/src/Containers/OhmmsPETE/tests/test_Matrix.cpp +++ b/src/Containers/OhmmsPETE/tests/test_Matrix.cpp @@ -2,9 +2,10 @@ // This file is distributed under the University of Illinois/NCSA Open Source License. // See LICENSE file in top directory for details. // -// Copyright (c) 2016 Jeongnim Kim and QMCPACK developers. +// Copyright (c) 2024 QMCPACK developers. // // File developed by: Mark Dewing, markdewing@gmail.com, University of Illinois at Urbana-Champaign +// Peter Doak, doakpw@ornl.gov, Oak Ridge National Lab // // File created by: Mark Dewing, markdewing@gmail.com, University of Illinois at Urbana-Champaign ////////////////////////////////////////////////////////////////////////////////////// @@ -109,6 +110,20 @@ TEST_CASE("matrix converting assignment", "[OhmmsPETE]") mat_A.assignUpperLeft(mat_D); CHECK(mat_A(1,0) == Approx(2.3)); CHECK(mat_A(1,2) == Approx(6.9)); + + Matrix mat_too_small(2,2); + CHECK_THROWS(mat_too_small = mat_A); + + Matrix mat_too_small_but_larger_value_type(2,2); + mat_too_small_but_larger_value_type = mat_B; + CHECK(mat_too_small_but_larger_value_type.rows() == 3); + CHECK(mat_too_small_but_larger_value_type.cols() == 3); + CHECK(mat_too_small_but_larger_value_type(1,1) == Approx(mat_B(1,1))); + Matrix too_small_but_same(2,2); + too_small_but_same = mat_A; + CHECK(too_small_but_same.rows() == 3); + CHECK(too_small_but_same.cols() == 3); + CHECK(too_small_but_same(1,1) == Approx(mat_A(1,1))); } } // namespace qmcplusplus