From 8859455f57305d9b9ea28da07114e30678a10232 Mon Sep 17 00:00:00 2001 From: David Bold Date: Mon, 5 Aug 2024 09:39:27 +0200 Subject: [PATCH 01/12] Allow lazy loading of mesh files This is needed to avoid loading the whole mesh file (gh#2900), which can be very big. This needs some changes in a few places, so probably worth to reconsider the design of the APIs, to make it a bit cleaner. --- include/bout/options.hxx | 8 ++++++ src/mesh/data/gridfromfile.cxx | 34 +++++++++++++++++++------ src/sys/options/options_netcdf.cxx | 40 ++++++++++++++++++++++-------- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index a0b745670f..a1d4cde8ca 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -53,6 +53,7 @@ class Options; #include #include +#include #include #include #include @@ -822,6 +823,13 @@ public: static std::string getDefaultSource(); + /// function to load a chunk of the data + std::unique_ptr(int xstart, int xend, int ystart, + int yend, int zstart, int zend)>> + lazyLoad{nullptr}; + bool is_loaded{true}; + std::vector shape; + private: /// The source label given to default values static const std::string DEFAULT_SOURCE; diff --git a/src/mesh/data/gridfromfile.cxx b/src/mesh/data/gridfromfile.cxx index 81faca3dc5..a96b2b0be0 100644 --- a/src/mesh/data/gridfromfile.cxx +++ b/src/mesh/data/gridfromfile.cxx @@ -175,7 +175,7 @@ bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, Options& option = data[name]; // Global (x, y, z) dimensions of field - const std::vector size = bout::utils::visit(GetDimensions{}, option.value); + std::vector size = bout::utils::visit(GetDimensions{}, option.value); switch (size.size()) { case 1: { @@ -194,6 +194,11 @@ bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, break; } case 3: { + if (not option.is_loaded) { + size[0] = option.shape[0]; + size[1] = option.shape[1]; + size[2] = option.shape[2]; + } // Check size if getting Field3D if constexpr (bout::utils::is_Field2D_v or bout::utils::is_FieldPerp_v) { output_warn.write( @@ -630,14 +635,27 @@ bool GridFile::readgrid_3dvar_real(const std::string& name, int yread, int ydest return false; } - const auto full_var = option.as>(); + if (not option.is_loaded) { + const auto& chunk = (*option.lazyLoad)(xread, xread + xsize - 1, yread, + yread + ysize - 1, 0, size[2] - 1); + for (int jx = 0; jx < xsize; jx++) { + for (int jy = 0; jy < ysize; jy++) { + for (int jz = 0; jz < size[2]; ++jz) { + var(jx + xdest, jy + ydest, jz) = chunk(jx, jy, jz); + } + } + } - for (int jx = xread; jx < xread + xsize; jx++) { - // jx is global x-index to start from - for (int jy = yread; jy < yread + ysize; jy++) { - // jy is global y-index to start from - for (int jz = 0; jz < size[2]; ++jz) { - var(jx - xread + xdest, jy - yread + ydest, jz) = full_var(jx, jy, jz); + } else { + const auto full_var = option.as>(); + + for (int jx = xread; jx < xread + xsize; jx++) { + // jx is global x-index to start from + for (int jy = yread; jy < yread + ysize; jy++) { + // jy is global y-index to start from + for (int jz = 0; jz < size[2]; ++jz) { + var(jx - xread + xdest, jy - yread + ydest, jz) = full_var(jx, jy, jz); + } } } } diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index 65fbca14c0..796ddd1e73 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -56,7 +56,8 @@ T readAttribute(const NcAtt& attribute) { return value; } -void readGroup(const std::string& filename, const NcGroup& group, Options& result) { +void readGroup(const std::string& filename, const NcGroup group, Options& result, + std::shared_ptr file) { // Iterate over all variables for (const auto& varpair : group.getVars()) { @@ -107,11 +108,30 @@ void readGroup(const std::string& filename, const NcGroup& group, Options& resul } case 3: { if (var_type == ncDouble or var_type == ncFloat) { - Tensor value(static_cast(dims[0].getSize()), - static_cast(dims[1].getSize()), - static_cast(dims[2].getSize())); - var.getVar(value.begin()); - result[var_name] = value; + Tensor dummy(0, 0, 0); + result[var_name] = dummy; + result[var_name].is_loaded = false; + result[var_name].shape = {dims[0].getSize(), dims[1].getSize(), + dims[2].getSize()}; + // We need to explicitly copy file, so that there is a pointer to the file, and + // the file does not get closed, which would prevent us from reading. + result[var_name].lazyLoad = + std::make_unique(int, int, int, int, int, int)>>( + [=, file](int xstart, int xend, int ystart, int yend, int zstart, + int zend) { + Tensor value(xend - xstart + 1, yend - ystart + 1, + zend - zstart + 1); + std::vector index; + index.push_back(xstart); + index.push_back(ystart); + index.push_back(zstart); + std::vector count; + count.push_back(xend - xstart + 1); + count.push_back(yend - ystart + 1); + count.push_back(zend - zstart + 1); + var.getVar(index, count, value.begin()); + return value; + }); } } } @@ -144,7 +164,7 @@ void readGroup(const std::string& filename, const NcGroup& group, Options& resul const auto& name = grouppair.first; const auto& subgroup = grouppair.second; - readGroup(filename, subgroup, result[name]); + readGroup(filename, subgroup, result[name], file); } } } // namespace @@ -155,14 +175,14 @@ Options OptionsNetCDF::read() { Timer timer("io"); // Open file - const NcFile read_file(filename, NcFile::read); + auto read_file = std::make_shared(filename, NcFile::read); - if (read_file.isNull()) { + if (read_file->isNull()) { throw BoutException("Could not open NetCDF file '{:s}' for reading", filename); } Options result; - readGroup(filename, read_file, result); + readGroup(filename, *read_file, result, read_file); return result; } From 2154d7c2b79b34f924d6a1959b80e99af78021ff Mon Sep 17 00:00:00 2001 From: David Bold Date: Mon, 5 Aug 2024 10:32:05 +0200 Subject: [PATCH 02/12] Remove warning from code It is perfectly fine to compile withut hypre. A runtime error with explanation is raised, if needed. --- include/bout/invert/laplacexy2_hypre.hxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/bout/invert/laplacexy2_hypre.hxx b/include/bout/invert/laplacexy2_hypre.hxx index 6a552e6b6e..9a488ed6ff 100644 --- a/include/bout/invert/laplacexy2_hypre.hxx +++ b/include/bout/invert/laplacexy2_hypre.hxx @@ -38,8 +38,6 @@ #if not BOUT_HAS_HYPRE // If no Hypre -#warning LaplaceXY requires Hypre. No LaplaceXY available - #include "bout/globalindexer.hxx" #include #include From a2a7f2adb24345601c01a4be8ad93f7818298efd Mon Sep 17 00:00:00 2001 From: David Bold Date: Mon, 5 Aug 2024 10:33:05 +0200 Subject: [PATCH 03/12] Print error if run fails --- tests/integrated/test-options-netcdf/runtest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrated/test-options-netcdf/runtest b/tests/integrated/test-options-netcdf/runtest index ab05ee1caf..db9adc7f29 100755 --- a/tests/integrated/test-options-netcdf/runtest +++ b/tests/integrated/test-options-netcdf/runtest @@ -5,7 +5,7 @@ # requires: not legacy_netcdf from boututils.datafile import DataFile -from boututils.run_wrapper import build_and_log, shell, launch +from boututils.run_wrapper import build_and_log, shell, launch_safe as launch from boutdata.data import BoutOptionsFile import math From acb97ee953a65de1688f57d2a48912b1395b2f7a Mon Sep 17 00:00:00 2001 From: David Bold Date: Mon, 5 Aug 2024 11:00:00 +0200 Subject: [PATCH 04/12] Make lazy loading optional Writing lazyly loaded files does not work, as for example the file is still locked from reading. --- include/bout/options_io.hxx | 2 +- src/physics/physicsmodel.cxx | 2 +- src/sys/options.cxx | 5 ++- src/sys/options/options_netcdf.cxx | 58 +++++++++++++++++------------- src/sys/options/options_netcdf.hxx | 2 +- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/include/bout/options_io.hxx b/include/bout/options_io.hxx index 57be8bbaae..59a063664c 100644 --- a/include/bout/options_io.hxx +++ b/include/bout/options_io.hxx @@ -61,7 +61,7 @@ public: OptionsIO& operator=(OptionsIO&&) noexcept = default; /// Read options from file - virtual Options read() = 0; + virtual Options read(bool lazy = true) = 0; /// Write options to file void write(const Options& options) { write(options, "t"); } diff --git a/src/physics/physicsmodel.cxx b/src/physics/physicsmodel.cxx index 9f538895ed..95de19b017 100644 --- a/src/physics/physicsmodel.cxx +++ b/src/physics/physicsmodel.cxx @@ -94,7 +94,7 @@ void PhysicsModel::initialise(Solver* s) { const bool restarting = Options::root()["restart"].withDefault(false); if (restarting) { - restart_options = restart_file->read(); + restart_options = restart_file->read(false); } // Call user init code to specify evolving variables diff --git a/src/sys/options.cxx b/src/sys/options.cxx index 1d18a0e462..c3ae902d40 100644 --- a/src/sys/options.cxx +++ b/src/sys/options.cxx @@ -540,7 +540,10 @@ Field3D Options::as(const Field3D& similar_to) const { } // Get a reference, to try and avoid copying - const auto& tensor = bout::utils::get>(value); + const auto& tensor = + is_loaded ? bout::utils::get>(value) + : (*lazyLoad)(0, localmesh->LocalNx - 1, 0, localmesh->LocalNy - 1, 0, + localmesh->LocalNz - 1); // Check if the dimension sizes are the same as a Field3D if (tensor.shape() diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index 796ddd1e73..29a2596f77 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -108,30 +108,38 @@ void readGroup(const std::string& filename, const NcGroup group, Options& result } case 3: { if (var_type == ncDouble or var_type == ncFloat) { - Tensor dummy(0, 0, 0); - result[var_name] = dummy; - result[var_name].is_loaded = false; - result[var_name].shape = {dims[0].getSize(), dims[1].getSize(), - dims[2].getSize()}; - // We need to explicitly copy file, so that there is a pointer to the file, and - // the file does not get closed, which would prevent us from reading. - result[var_name].lazyLoad = - std::make_unique(int, int, int, int, int, int)>>( - [=, file](int xstart, int xend, int ystart, int yend, int zstart, + if (file) { + Tensor dummy(0, 0, 0); + result[var_name] = dummy; + result[var_name].is_loaded = false; + result[var_name].shape = {dims[0].getSize(), dims[1].getSize(), + dims[2].getSize()}; + // We need to explicitly copy file, so that there is a pointer to the file, and + // the file does not get closed, which would prevent us from reading. + result[var_name].lazyLoad = std::make_unique< + std::function(int, int, int, int, int, int)>>( + [file, var](int xstart, int xend, int ystart, int yend, int zstart, int zend) { - Tensor value(xend - xstart + 1, yend - ystart + 1, - zend - zstart + 1); - std::vector index; - index.push_back(xstart); - index.push_back(ystart); - index.push_back(zstart); - std::vector count; - count.push_back(xend - xstart + 1); - count.push_back(yend - ystart + 1); - count.push_back(zend - zstart + 1); - var.getVar(index, count, value.begin()); - return value; - }); + Tensor value(xend - xstart + 1, yend - ystart + 1, + zend - zstart + 1); + std::vector index; + index.push_back(xstart); + index.push_back(ystart); + index.push_back(zstart); + std::vector count; + count.push_back(xend - xstart + 1); + count.push_back(yend - ystart + 1); + count.push_back(zend - zstart + 1); + var.getVar(index, count, value.begin()); + return value; + }); + } else { + Tensor value(static_cast(dims[0].getSize()), + static_cast(dims[1].getSize()), + static_cast(dims[2].getSize())); + var.getVar(value.begin()); + result[var_name] = value; + } } } } @@ -171,7 +179,7 @@ void readGroup(const std::string& filename, const NcGroup group, Options& result namespace bout { -Options OptionsNetCDF::read() { +Options OptionsNetCDF::read(bool lazy) { Timer timer("io"); // Open file @@ -182,7 +190,7 @@ Options OptionsNetCDF::read() { } Options result; - readGroup(filename, *read_file, result, read_file); + readGroup(filename, *read_file, result, lazy ? read_file : nullptr); return result; } diff --git a/src/sys/options/options_netcdf.hxx b/src/sys/options/options_netcdf.hxx index 8f195c9d92..ba81e0d8d8 100644 --- a/src/sys/options/options_netcdf.hxx +++ b/src/sys/options/options_netcdf.hxx @@ -47,7 +47,7 @@ public: OptionsNetCDF& operator=(OptionsNetCDF&&) noexcept = default; /// Read options from file - Options read(); + Options read(bool lazy = true); /// Write options to file void write(const Options& options) { write(options, "t"); } From 9c83ea1987b65e14e2bd6a792f6d7f2e5f04d657 Mon Sep 17 00:00:00 2001 From: David Bold Date: Tue, 15 Oct 2024 15:37:44 +0200 Subject: [PATCH 05/12] Apply suggestions from code review * make lazyLoad variable private * provide setter and way to call it * is_loaded can be a const function * vector can be constructed in place (and thus const) --- include/bout/options.hxx | 18 +++++++++++++----- src/mesh/data/gridfromfile.cxx | 8 ++++---- src/sys/options.cxx | 6 +++--- src/sys/options/options_netcdf.cxx | 16 +++++----------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index a1d4cde8ca..015ba83bf3 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -823,11 +823,17 @@ public: static std::string getDefaultSource(); - /// function to load a chunk of the data - std::unique_ptr(int xstart, int xend, int ystart, - int yend, int zstart, int zend)>> - lazyLoad{nullptr}; - bool is_loaded{true}; + /// API for delayed loading of data from the grid file + /// Currently only for 3D data + using lazyLoadFunction = std::unique_ptr( + int xstart, int xend, int ystart, int yend, int zstart, int zend)>>; + void setLazyLoad(lazyLoadFunction func) {lazyLoad = std::move(func); } + Tensor doLazyLoad(int xstart, int xend, int ystart, int yend, int zstart, + int zend) const { + ASSERT1(lazyLoad != nullptr); + return (*lazyLoad)(xstart, xend, ystart, yend, zstart, zend); + } + bool is_loaded() const { return lazyLoad == nullptr; } std::vector shape; private: @@ -842,6 +848,8 @@ private: std::map children; ///< If a section then has children mutable bool value_used = false; ///< Record whether this value is used + lazyLoadFunction lazyLoad{nullptr}; + template void _set_no_check(T val, std::string source) { if (not children.empty()) { diff --git a/src/mesh/data/gridfromfile.cxx b/src/mesh/data/gridfromfile.cxx index a96b2b0be0..7ffc6afe90 100644 --- a/src/mesh/data/gridfromfile.cxx +++ b/src/mesh/data/gridfromfile.cxx @@ -194,7 +194,7 @@ bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, break; } case 3: { - if (not option.is_loaded) { + if (not option.is_loaded()) { size[0] = option.shape[0]; size[1] = option.shape[1]; size[2] = option.shape[2]; @@ -635,9 +635,9 @@ bool GridFile::readgrid_3dvar_real(const std::string& name, int yread, int ydest return false; } - if (not option.is_loaded) { - const auto& chunk = (*option.lazyLoad)(xread, xread + xsize - 1, yread, - yread + ysize - 1, 0, size[2] - 1); + if (not option.is_loaded()) { + const auto& chunk = option.doLazyLoad(xread, xread + xsize - 1, yread, + yread + ysize - 1, 0, size[2] - 1); for (int jx = 0; jx < xsize; jx++) { for (int jy = 0; jy < ysize; jy++) { for (int jz = 0; jz < size[2]; ++jz) { diff --git a/src/sys/options.cxx b/src/sys/options.cxx index c3ae902d40..8563b6e45f 100644 --- a/src/sys/options.cxx +++ b/src/sys/options.cxx @@ -541,9 +541,9 @@ Field3D Options::as(const Field3D& similar_to) const { // Get a reference, to try and avoid copying const auto& tensor = - is_loaded ? bout::utils::get>(value) - : (*lazyLoad)(0, localmesh->LocalNx - 1, 0, localmesh->LocalNy - 1, 0, - localmesh->LocalNz - 1); + is_loaded() ? bout::utils::get>(value) + : doLazyLoad(0, localmesh->LocalNx - 1, 0, localmesh->LocalNy - 1, 0, + localmesh->LocalNz - 1); // Check if the dimension sizes are the same as a Field3D if (tensor.shape() diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index 29a2596f77..a4b886d309 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -111,28 +111,22 @@ void readGroup(const std::string& filename, const NcGroup group, Options& result if (file) { Tensor dummy(0, 0, 0); result[var_name] = dummy; - result[var_name].is_loaded = false; result[var_name].shape = {dims[0].getSize(), dims[1].getSize(), dims[2].getSize()}; // We need to explicitly copy file, so that there is a pointer to the file, and // the file does not get closed, which would prevent us from reading. - result[var_name].lazyLoad = std::make_unique< + result[var_name].setLazyLoad(std::make_unique< std::function(int, int, int, int, int, int)>>( [file, var](int xstart, int xend, int ystart, int yend, int zstart, int zend) { Tensor value(xend - xstart + 1, yend - ystart + 1, zend - zstart + 1); - std::vector index; - index.push_back(xstart); - index.push_back(ystart); - index.push_back(zstart); - std::vector count; - count.push_back(xend - xstart + 1); - count.push_back(yend - ystart + 1); - count.push_back(zend - zstart + 1); + const std::vector index{xstart, ystart, zstart}; + const std::vector count{(xend - xstart + 1), (yend - ystart + 1), + (zend - zstart + 1)}; var.getVar(index, count, value.begin()); return value; - }); + })); } else { Tensor value(static_cast(dims[0].getSize()), static_cast(dims[1].getSize()), From c48d6094d2d2dbb86f09ee028bc8ae32b8bd3fd7 Mon Sep 17 00:00:00 2001 From: David Bold Date: Tue, 15 Oct 2024 15:41:08 +0200 Subject: [PATCH 06/12] Avoid implicit cast Use a helper function, that checks the range is valid, and if not throws. --- src/sys/options/options_netcdf.cxx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index a4b886d309..0452285f49 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -115,15 +115,22 @@ void readGroup(const std::string& filename, const NcGroup group, Options& result dims[2].getSize()}; // We need to explicitly copy file, so that there is a pointer to the file, and // the file does not get closed, which would prevent us from reading. - result[var_name].setLazyLoad(std::make_unique< - std::function(int, int, int, int, int, int)>>( + result[var_name].setLazyLoad(std::make_unique( + int, int, int, int, int, int)>>( [file, var](int xstart, int xend, int ystart, int yend, int zstart, int zend) { + const auto i2s = [](int i) { + if (i < 0) { + throw BoutException("BadCast {} < 0", i); + } + return static_cast(i); + }; Tensor value(xend - xstart + 1, yend - ystart + 1, zend - zstart + 1); - const std::vector index{xstart, ystart, zstart}; - const std::vector count{(xend - xstart + 1), (yend - ystart + 1), - (zend - zstart + 1)}; + const std::vector index{i2s(xstart), i2s(ystart), i2s(zstart)}; + const std::vector count{i2s(xend - xstart + 1), + i2s(yend - ystart + 1), + i2s(zend - zstart + 1)}; var.getVar(index, count, value.begin()); return value; })); From 18c7b42a0c5d75ca2d34961926546c9e6257d6ba Mon Sep 17 00:00:00 2001 From: David Bold Date: Tue, 15 Oct 2024 16:26:39 +0200 Subject: [PATCH 07/12] move getShape to options --- include/bout/options.hxx | 6 ++++- src/mesh/data/gridfromfile.cxx | 39 +++++------------------------- src/sys/options.cxx | 29 ++++++++++++++++++++++ src/sys/options/options_netcdf.cxx | 14 ++++++++--- 4 files changed, 50 insertions(+), 38 deletions(-) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index 015ba83bf3..a7b93528ed 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -834,7 +834,8 @@ public: return (*lazyLoad)(xstart, xend, ystart, yend, zstart, zend); } bool is_loaded() const { return lazyLoad == nullptr; } - std::vector shape; + std::vector getShape() const; + void setLazyShape(std::vector shape) { lazy_shape = shape; } private: /// The source label given to default values @@ -848,7 +849,10 @@ private: std::map children; ///< If a section then has children mutable bool value_used = false; ///< Record whether this value is used + // Function to load data lazyLoadFunction lazyLoad{nullptr}; + // Shape of underlying data + std::vector lazy_shape; template void _set_no_check(T val, std::string source) { diff --git a/src/mesh/data/gridfromfile.cxx b/src/mesh/data/gridfromfile.cxx index 7ffc6afe90..3152ebced9 100644 --- a/src/mesh/data/gridfromfile.cxx +++ b/src/mesh/data/gridfromfile.cxx @@ -131,28 +131,6 @@ bool GridFile::get(Mesh* m, Field3D& var, const std::string& name, BoutReal def, return getField(m, var, name, def, location); } -namespace { -/// Visitor that returns the shape of its argument -struct GetDimensions { - std::vector operator()([[maybe_unused]] bool value) { return {1}; } - std::vector operator()([[maybe_unused]] int value) { return {1}; } - std::vector operator()([[maybe_unused]] BoutReal value) { return {1}; } - std::vector operator()([[maybe_unused]] const std::string& value) { return {1}; } - std::vector operator()(const Array& array) { return {array.size()}; } - std::vector operator()(const Matrix& array) { - const auto shape = array.shape(); - return {std::get<0>(shape), std::get<1>(shape)}; - } - std::vector operator()(const Tensor& array) { - const auto shape = array.shape(); - return {std::get<0>(shape), std::get<1>(shape), std::get<2>(shape)}; - } - std::vector operator()(const Field& array) { - return {array.getNx(), array.getNy(), array.getNz()}; - } -}; -} // namespace - template bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, CELL_LOC location) { @@ -175,7 +153,7 @@ bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, Options& option = data[name]; // Global (x, y, z) dimensions of field - std::vector size = bout::utils::visit(GetDimensions{}, option.value); + const std::vector size = option.getShape(); switch (size.size()) { case 1: { @@ -194,11 +172,6 @@ bool GridFile::getField(Mesh* m, T& var, const std::string& name, BoutReal def, break; } case 3: { - if (not option.is_loaded()) { - size[0] = option.shape[0]; - size[1] = option.shape[1]; - size[2] = option.shape[2]; - } // Check size if getting Field3D if constexpr (bout::utils::is_Field2D_v or bout::utils::is_FieldPerp_v) { output_warn.write( @@ -504,7 +477,7 @@ bool GridFile::get(Mesh* UNUSED(m), std::vector& var, const std::strin bool GridFile::hasXBoundaryGuards(Mesh* m) { // Global (x,y) dimensions of some field // a grid file should always contain "dx" - const std::vector size = bout::utils::visit(GetDimensions{}, data["dx"].value); + const std::vector size = data["dx"].getShape(); if (size.empty()) { // handle case where "dx" is not present - non-standard grid file @@ -539,7 +512,7 @@ bool GridFile::readgrid_3dvar_fft(Mesh* m, const std::string& name, int yread, i } /// Check the size of the data - const std::vector size = bout::utils::visit(GetDimensions{}, data[name].value); + const std::vector size = data[name].getShape(); if (size.size() != 3) { output_warn.write("\tWARNING: Number of dimensions of {:s} incorrect\n", name); @@ -628,7 +601,7 @@ bool GridFile::readgrid_3dvar_real(const std::string& name, int yread, int ydest Options& option = data[name]; /// Check the size of the data - const std::vector size = bout::utils::visit(GetDimensions{}, option.value); + const std::vector size = option.getShape(); if (size.size() != 3) { output_warn.write("\tWARNING: Number of dimensions of {:s} incorrect\n", name); @@ -682,7 +655,7 @@ bool GridFile::readgrid_perpvar_fft(Mesh* m, const std::string& name, int xread, /// Check the size of the data Options& option = data[name]; - const std::vector size = bout::utils::visit(GetDimensions{}, option.value); + const std::vector size = option.getShape(); if (size.size() != 2) { output_warn.write("\tWARNING: Number of dimensions of {:s} incorrect\n", name); @@ -765,7 +738,7 @@ bool GridFile::readgrid_perpvar_real(const std::string& name, int xread, int xde /// Check the size of the data Options& option = data[name]; - const std::vector size = bout::utils::visit(GetDimensions{}, option.value); + const std::vector size = option.getShape(); if (size.size() != 2) { output_warn.write("\tWARNING: Number of dimensions of {:s} incorrect\n", name); diff --git a/src/sys/options.cxx b/src/sys/options.cxx index 8563b6e45f..f550749404 100644 --- a/src/sys/options.cxx +++ b/src/sys/options.cxx @@ -932,6 +932,35 @@ std::vector Options::getFlattenedKeys() const { return flattened_names; } +namespace { +/// Visitor that returns the shape of its argument +struct GetDimensions { + std::vector operator()([[maybe_unused]] bool value) { return {1}; } + std::vector operator()([[maybe_unused]] int value) { return {1}; } + std::vector operator()([[maybe_unused]] BoutReal value) { return {1}; } + std::vector operator()([[maybe_unused]] const std::string& value) { return {1}; } + std::vector operator()(const Array& array) { return {array.size()}; } + std::vector operator()(const Matrix& array) { + const auto shape = array.shape(); + return {std::get<0>(shape), std::get<1>(shape)}; + } + std::vector operator()(const Tensor& array) { + const auto shape = array.shape(); + return {std::get<0>(shape), std::get<1>(shape), std::get<2>(shape)}; + } + std::vector operator()(const Field& array) { + return {array.getNx(), array.getNy(), array.getNz()}; + } +}; +} // namespace + +std::vector Options::getShape() const { + if (is_loaded()) { + return bout::utils::visit(GetDimensions{}, value); + } + return lazy_shape; +} + fmt::format_parse_context::iterator bout::details::OptionsFormatterBase::parse(fmt::format_parse_context& ctx) { diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index 0452285f49..8b0b81480e 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -9,6 +9,7 @@ #include "bout/mesh.hxx" #include "bout/sys/timer.hxx" +#include #include #include #include @@ -109,10 +110,15 @@ void readGroup(const std::string& filename, const NcGroup group, Options& result case 3: { if (var_type == ncDouble or var_type == ncFloat) { if (file) { - Tensor dummy(0, 0, 0); - result[var_name] = dummy; - result[var_name].shape = {dims[0].getSize(), dims[1].getSize(), - dims[2].getSize()}; + result[var_name] = Tensor(0, 0, 0); + const auto s2i = [](size_t s) { + if (s > INT_MAX) { + throw BoutException("BadCast {} > {}", s, INT_MAX); + } + return static_cast(s); + }; + result[var_name].setLazyShape( + {s2i(dims[0].getSize()), s2i(dims[1].getSize()), s2i(dims[2].getSize())}); // We need to explicitly copy file, so that there is a pointer to the file, and // the file does not get closed, which would prevent us from reading. result[var_name].setLazyLoad(std::make_unique( From 08b3962641616d0db7d1bc60118b9064c3bdcd11 Mon Sep 17 00:00:00 2001 From: dschwoerer Date: Tue, 15 Oct 2024 14:27:15 +0000 Subject: [PATCH 08/12] Apply clang-format changes --- include/bout/options.hxx | 2 +- src/sys/options.cxx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index a7b93528ed..849da76321 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -827,7 +827,7 @@ public: /// Currently only for 3D data using lazyLoadFunction = std::unique_ptr( int xstart, int xend, int ystart, int yend, int zstart, int zend)>>; - void setLazyLoad(lazyLoadFunction func) {lazyLoad = std::move(func); } + void setLazyLoad(lazyLoadFunction func) { lazyLoad = std::move(func); } Tensor doLazyLoad(int xstart, int xend, int ystart, int yend, int zstart, int zend) const { ASSERT1(lazyLoad != nullptr); diff --git a/src/sys/options.cxx b/src/sys/options.cxx index f550749404..bde6b902d7 100644 --- a/src/sys/options.cxx +++ b/src/sys/options.cxx @@ -541,9 +541,9 @@ Field3D Options::as(const Field3D& similar_to) const { // Get a reference, to try and avoid copying const auto& tensor = - is_loaded() ? bout::utils::get>(value) - : doLazyLoad(0, localmesh->LocalNx - 1, 0, localmesh->LocalNy - 1, 0, - localmesh->LocalNz - 1); + is_loaded() ? bout::utils::get>(value) + : doLazyLoad(0, localmesh->LocalNx - 1, 0, localmesh->LocalNy - 1, 0, + localmesh->LocalNz - 1); // Check if the dimension sizes are the same as a Field3D if (tensor.shape() From d957ed69860f98429c410c51da6ebdf4dd43c6a6 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 18 Oct 2024 09:52:48 +0200 Subject: [PATCH 09/12] shape is not needed afterwards, so we can std::move Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- include/bout/options.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index 849da76321..3277edf437 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -835,7 +835,7 @@ public: } bool is_loaded() const { return lazyLoad == nullptr; } std::vector getShape() const; - void setLazyShape(std::vector shape) { lazy_shape = shape; } + void setLazyShape(std::vector shape) { lazy_shape = std::move(shape); } private: /// The source label given to default values From 241fa070ef5ec1ffd4bb74a14e63e62ea2d9734b Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 18 Oct 2024 13:28:48 +0200 Subject: [PATCH 10/12] Add some documentation --- include/bout/options.hxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/bout/options.hxx b/include/bout/options.hxx index 3277edf437..e04d66e585 100644 --- a/include/bout/options.hxx +++ b/include/bout/options.hxx @@ -828,12 +828,17 @@ public: using lazyLoadFunction = std::unique_ptr( int xstart, int xend, int ystart, int yend, int zstart, int zend)>>; void setLazyLoad(lazyLoadFunction func) { lazyLoad = std::move(func); } + /// Load and get a chunk of the data Tensor doLazyLoad(int xstart, int xend, int ystart, int yend, int zstart, int zend) const { ASSERT1(lazyLoad != nullptr); return (*lazyLoad)(xstart, xend, ystart, yend, zstart, zend); } + /// Some backends support to only read the data when needed. This + /// allows to check whether the data is loaded, or whether it needs + /// to be loaded by doLazyLoad. bool is_loaded() const { return lazyLoad == nullptr; } + /// Get the shape of the value std::vector getShape() const; void setLazyShape(std::vector shape) { lazy_shape = std::move(shape); } From 64fadf7223f59b962aaba47e0e911b34348fd415 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 18 Oct 2024 09:51:00 +0200 Subject: [PATCH 11/12] Prefer const-ref --- src/sys/options/options_netcdf.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sys/options/options_netcdf.cxx b/src/sys/options/options_netcdf.cxx index 8b0b81480e..873f143d6b 100644 --- a/src/sys/options/options_netcdf.cxx +++ b/src/sys/options/options_netcdf.cxx @@ -57,8 +57,8 @@ T readAttribute(const NcAtt& attribute) { return value; } -void readGroup(const std::string& filename, const NcGroup group, Options& result, - std::shared_ptr file) { +void readGroup(const std::string& filename, const NcGroup& group, Options& result, + const std::shared_ptr& file) { // Iterate over all variables for (const auto& varpair : group.getVars()) { From 4e2f00682a239d58541cb57b4c86a19bd1b451d5 Mon Sep 17 00:00:00 2001 From: David Bold Date: Fri, 18 Oct 2024 09:51:18 +0200 Subject: [PATCH 12/12] Add override --- src/sys/options/options_netcdf.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sys/options/options_netcdf.hxx b/src/sys/options/options_netcdf.hxx index ba81e0d8d8..192f894d52 100644 --- a/src/sys/options/options_netcdf.hxx +++ b/src/sys/options/options_netcdf.hxx @@ -47,7 +47,7 @@ public: OptionsNetCDF& operator=(OptionsNetCDF&&) noexcept = default; /// Read options from file - Options read(bool lazy = true); + Options read(bool lazy = true) override; /// Write options to file void write(const Options& options) { write(options, "t"); }