diff --git a/resolve/matrix/io.cpp b/resolve/matrix/io.cpp index b48e34b63..600b5a5e3 100644 --- a/resolve/matrix/io.cpp +++ b/resolve/matrix/io.cpp @@ -1,11 +1,13 @@ #include "io.hpp" +#include #include #include #include #include #include #include +#include #include #include @@ -148,10 +150,11 @@ namespace ReSolve matrix::Sparse* A, std::list& tmp); // static void print_list(std::list& l); - static int loadToList(std::istream& file, bool is_expand_symmetric, std::list& tmp); - static int removeDuplicates(std::list& tmp); - static int copyListToCoo(const std::list& tmp, matrix::Coo* A); - static int copyListToCsr(const std::list& tmp, matrix::Csr* A); + static int loadToList(std::istream& file, bool is_expand_symmetric, std::list& tmp); + static int removeDuplicates(std::list& tmp); + static int copyListToCoo(const std::list& tmp, matrix::Coo* A); + static int copyListToCsr(const std::list& tmp, matrix::Csr* A); + static bool readArrayBody(std::istream& file, index_type n, std::vector& values); /** * @brief Create a COO matrix and populate it with data from Matrix Market @@ -254,7 +257,6 @@ namespace ReSolve std::stringstream ss; std::string line; - index_type i = 0; index_type n, m; std::getline(file, line); @@ -265,13 +267,14 @@ namespace ReSolve ss << line; ss >> n >> m; - real_type* vec = new real_type[n]; - real_type a; - while (file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - vec[i] = a; - i++; + return nullptr; } + + real_type* vec = new real_type[n]; + std::copy(values.begin(), values.end(), vec); return vec; } @@ -285,7 +288,6 @@ namespace ReSolve std::stringstream ss; std::string line; - index_type i = 0; index_type n, m; std::getline(file, line); @@ -296,14 +298,15 @@ namespace ReSolve ss << line; ss >> n >> m; - vector::Vector* vec = new vector::Vector(n); - vec->allocate(memory::HOST); - real_type a; - while (file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - vec->getData(memory::HOST)[i] = a; - i++; + return nullptr; } + + vector::Vector* vec = new vector::Vector(n); + vec->allocate(memory::HOST); + std::copy(values.begin(), values.end(), vec->getData(memory::HOST)); vec->setDataUpdated(memory::HOST); return vec; } @@ -402,17 +405,18 @@ namespace ReSolve ss << line; ss >> n >> m; - if (rhs == nullptr) + std::vector values; + if (!readArrayBody(file, n, values)) { - rhs = new real_type[n]; + Logger::error() << "Array not updated!\n"; + return; } - real_type a; - index_type i = 0; - while (file >> a) + + if (rhs == nullptr) { - rhs[i] = a; - i++; + rhs = new real_type[n]; } + std::copy(values.begin(), values.end(), rhs); } void updateVectorFromFile(std::istream& file, vector::Vector* vec_rhs) @@ -443,15 +447,14 @@ namespace ReSolve return; } - real_type* rhs = vec_rhs->getData(memory::HOST); - real_type a = 0.0; - index_type i = 0; - while (file >> a) + std::vector values; + if (!readArrayBody(file, n, values)) { - rhs[i] = a; - // std::cout << i << ": " << a << "\n"; - i++; + Logger::error() << "Vector not updated!\n"; + return; } + + std::copy(values.begin(), values.end(), vec_rhs->getData(memory::HOST)); vec_rhs->setDataUpdated(memory::HOST); } @@ -521,6 +524,55 @@ namespace ReSolve // // Static helper functions + + /** + * @brief Reads the body of a Matrix Market vector into `values`. + * + * The header says how many entries the body has. Reading fewer or more + * than that means the file is malformed, so nothing is returned to the + * caller in that case. Trailing blank lines are not entries; the + * extraction operator skips them. + * + * @param file - input stream positioned at the first entry. + * @param n - number of entries declared in the header. + * @param values - output, filled only when the body matches the header. + * @return true if the body holds exactly `n` entries. + */ + static bool readArrayBody(std::istream& file, index_type n, std::vector& values) + { + values.clear(); + if (n < 0) + { + Logger::error() << "Matrix market header declares a negative length " << n << ".\n"; + return false; + } + values.reserve(static_cast(n)); + + real_type a; + while (values.size() < static_cast(n) && file >> a) + { + values.push_back(a); + } + + if (values.size() < static_cast(n)) + { + Logger::error() << "Matrix market header declares " << n << " entries but only " + << values.size() << " were read.\n"; + values.clear(); + return false; + } + + if (file >> a) + { + Logger::error() << "Matrix market header declares " << n + << " entries but the body has more.\n"; + values.clear(); + return false; + } + + return true; + } + // /** diff --git a/tests/unit/matrix/MatrixIoTests.hpp b/tests/unit/matrix/MatrixIoTests.hpp index e5505e31c..8c42a1aa6 100644 --- a/tests/unit/matrix/MatrixIoTests.hpp +++ b/tests/unit/matrix/MatrixIoTests.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -454,6 +455,96 @@ namespace ReSolve return status.report(__func__); } + /** + * @brief Reject a vector file whose body does not match its header. + * + * The header of a Matrix Market vector says how many entries follow. + * A file with fewer or more than that is malformed, so the readers + * return nothing and leave the caller's data alone. + */ + TestOutcome rhsVectorLengthMismatch() + { + TestStatus status; + status = true; + + // These cases all log an error on purpose, so keep the output quiet. + ReSolve::io::Logger::setVerbosity(ReSolve::io::Logger::NONE); + + const std::string bad_files[2] = {short_vector_file_, long_vector_file_}; + const char* bad_names[2] = {"short", "long"}; + + for (int k = 0; k < 2; ++k) + { + std::istringstream array_file(bad_files[k]); + real_type* rhs_array = ReSolve::io::createArrayFromFile(array_file); + if (rhs_array != nullptr) + { + std::cout << "createArrayFromFile accepted a " << bad_names[k] << " body.\n"; + status = false; + delete[] rhs_array; + } + + std::istringstream vector_file(bad_files[k]); + vector::Vector* rhs_vector = ReSolve::io::createVectorFromFile(vector_file); + if (rhs_vector != nullptr) + { + std::cout << "createVectorFromFile accepted a " << bad_names[k] << " body.\n"; + status = false; + delete rhs_vector; + } + + // An update leaves the target untouched, so the sentinel survives. + index_type N = static_cast(general_vector_vals_.size()); + vector::Vector vec_rhs(N); + vec_rhs.allocate(memory::HOST); + real_type* data = vec_rhs.getData(memory::HOST); + for (index_type i = 0; i < N; ++i) + { + data[i] = -1.0; + } + + std::istringstream update_file(bad_files[k]); + ReSolve::io::updateVectorFromFile(update_file, &vec_rhs); + for (index_type i = 0; i < N; ++i) + { + if (!isEqual(vec_rhs.getData(memory::HOST)[i], -1.0)) + { + std::cout << "updateVectorFromFile wrote data from a " << bad_names[k] + << " body at element " << i << ".\n"; + status = false; + break; + } + } + } + + ReSolve::io::Logger::setVerbosity(ReSolve::io::Logger::WARNINGS); + + // Blank lines after the last entry are not entries. + std::istringstream padded_file(padded_vector_file_); + vector::Vector* padded = ReSolve::io::createVectorFromFile(padded_file); + if (padded == nullptr) + { + std::cout << "createVectorFromFile rejected a file with trailing blank lines.\n"; + status = false; + } + else + { + const real_type* padded_data = padded->getData(memory::HOST); + for (size_t i = 0; i < general_vector_vals_.size(); ++i) + { + if (!isEqual(padded_data[i], general_vector_vals_[i])) + { + std::cout << "Incorrect vector value at storage element " << i << ".\n"; + status = false; + break; + } + } + delete padded; + } + + return status.report(__func__); + } + private: bool verifyAnswer(/* const */ ReSolve::matrix::Coo& answer, const std::vector& row_data, @@ -737,6 +828,41 @@ namespace ReSolve 3.000e-02 4.000e+00 5.505e+02 +)"; + + /// Header says five entries, body has three. + const std::string short_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 +)"; + + /// Header says five entries, body has seven. + const std::string long_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 + 4.000e+00 + 5.505e+02 + 6.000e+00 + 7.000e+00 +)"; + + /// Five entries, then blank lines that are not entries. + const std::string padded_vector_file_ = + R"(%%MatrixMarket matrix array real general + 5 1 + 1.000e+00 + 2.000e+01 + 3.000e-02 + 4.000e+00 + 5.505e+02 + + )"; const std::vector general_vector_vals_ = {1.000e+00, diff --git a/tests/unit/matrix/runMatrixIoTests.cpp b/tests/unit/matrix/runMatrixIoTests.cpp index 7915b0909..f3b2f57e4 100644 --- a/tests/unit/matrix/runMatrixIoTests.cpp +++ b/tests/unit/matrix/runMatrixIoTests.cpp @@ -22,6 +22,7 @@ int main(int, char**) result += test.rhsVectorReadFromFile(); result += test.rhsArrayReadAndUpdate(); result += test.rhsVectorReadAndUpdate(); + result += test.rhsVectorLengthMismatch(); return result.summary(); }