Skip to content

Add Array/Matrix/Tensor<int> to options, additional limiters for grids (rebase)#2767

Merged
ZedThree merged 19 commits into
nextfrom
options-array-ints-rebase
Nov 19, 2025
Merged

Add Array/Matrix/Tensor<int> to options, additional limiters for grids (rebase)#2767
ZedThree merged 19 commits into
nextfrom
options-array-ints-rebase

Conversation

@bendudson
Copy link
Copy Markdown
Contributor

Rebase of #2721 on next

  • Adds Array to the Options type
  • Reads Array from NetCDF files (e.g. mesh input file), and makes them available through Mesh::get

The reason this was added is to be able to include additional limiters in the input file. The format is a short-term solution only, though these have a habit of becoming permanent.

bendudson and others added 4 commits October 12, 2023 08:26
Enable 1D arrays of integers to be read from NetCDF files,
and made available via `Mesh::get()`. Mainly useful for
mesh construction.
When reading 1D vector of ints, check that the input is long enough,
and resize the output vector.
A short-term fix, allowing extra limiters to be added with inputs:

limiter_count : int
limiter_yinds : [int], length limiter_count
limiter_xstarts : [int], length limiter_count
limiter_xends : [int], length limiter_count

The limiter(s) are added between yinds[i] and yinds[i] + 1
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/impls/bout/boutmesh.cxx Outdated
int limiter_count = 0;
Mesh::get(limiter_count, "limiter_count", 0);
if (limiter_count > 0) {
std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;
std::vector<int> limiter_yinds;
std::vector<int> limiter_xstarts;
std::vector<int> limiter_xends;

Comment thread src/mesh/impls/bout/boutmesh.cxx Outdated
Comment thread src/mesh/impls/bout/boutmesh.cxx Outdated
Comment thread src/mesh/impls/bout/boutmesh.cxx Outdated
dschwoerer
dschwoerer previously approved these changes Oct 13, 2023
Copy link
Copy Markdown
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test would be nice 😊

@bendudson bendudson changed the title Add Array<int> to options, additional limiters for grids (rebase) Add Array/Matrix/Tensor<int> to options, additional limiters for grids (rebase) May 30, 2025
bendudson and others added 5 commits June 4, 2025 11:59
Without these the build fails at link time when building with ADIOS2.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* next: (247 commits)
  Remove vagrant config
  Remove makefiles
  Remove configure left overs
  Avoid mpi-compatibility issues
  BoutComm::get() might return int
  Bump bundled fmt
  Delete v5 header shims
  CI: Fedora container is already fully setup
  Apply clang-format changes
  Add `has_hypre` to python `boutconfig`
  Update examples for LaplaceXY factory
  Fix call to virtual function in ctor
  CI: Don't warn about including PETSc or MPI symbols directly
  Try to appease clang-tidy `misc-include-cleaner`
  Remove private variables from base class
  Bump soname
  Fix typo in define
  Apply clang-format changes
  Add factory for LaplaceXY, LaplaceXY2, LaplaceXY2Hypre
  Enable non-const iteration over `Options`
  ...
ZedThree
ZedThree previously approved these changes Nov 18, 2025
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread include/bout/options.hxx
Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/data/gridfromfile.cxx
Comment thread src/mesh/impls/bout/boutmesh.cxx Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread include/bout/traits.hxx

/// Create a `std::index_sequence` with the length of a tuple ``T``
template <typename T>
using tuple_index_sequence = std::make_index_sequence<std::tuple_size_v<T>>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "std::make_index_sequence" is directly included [misc-include-cleaner]

include/bout/traits.hxx:5:

+ #include <utility>

@@ -1,4 +1,5 @@
#include "bout/build_defines.hxx"
#include "bout/traits.hxx"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: included header traits.hxx is not used directly [misc-include-cleaner]

Suggested change


using bout::utils::tuple_index_sequence;

template <class Tuple, std::size_t... I>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "std::size_t" is directly included [misc-include-cleaner]

template <class Tuple, std::size_t... I>
                            ^

using bout::utils::tuple_index_sequence;

template <class Tuple, std::size_t... I>
auto make_dims_impl(NcGroup& group, Tuple&& t, std::index_sequence<I...> /* index */) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "std::index_sequence" is directly included [misc-include-cleaner]

src/sys/options/options_netcdf.cxx:1:

+ #include <utility>

template <class Tuple, std::size_t... I>
auto make_dims_impl(NcGroup& group, Tuple&& t, std::index_sequence<I...> /* index */) {
return std::vector{findDimension(group, fmt::format("dim_{}", I),
std::get<I>(std::forward<Tuple>(t)))...};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 't' used after it was forwarded [bugprone-use-after-move]

                                   std::get<I>(std::forward<Tuple>(t)))...};
                                                                   ^
Additional context

src/sys/options/options_netcdf.cxx:307: forward occurred here

                                   std::get<I>(std::forward<Tuple>(t)))...};
                                   ^

Comment thread src/sys/type_name.cxx
return "Array<int>";
}
template <>
std::string typeName<Array<BoutReal>>() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "Array" is directly included [misc-include-cleaner]

std::string typeName<Array<BoutReal>>() {
                     ^

}
}

Options fields{{"f2d", Field2D(1.0)},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "Field2D" is directly included [misc-include-cleaner]

tests/integrated/test-options-adios/test-options-adios.cxx:3:

- #include "bout/options_io.hxx"
+ #include "bout/field2d.hxx"
+ #include "bout/options_io.hxx"

}
}

Options fields{{"f2d", Field2D(1.0)},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'fields' of type 'Options' can be declared 'const' [misc-const-correctness]

Suggested change
Options fields{{"f2d", Field2D(1.0)},
Options const fields{{"f2d", Field2D(1.0)},

}

Options fields{{"f2d", Field2D(1.0)},
{"f3d", Field3D(2.0)},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "Field3D" is directly included [misc-include-cleaner]

tests/integrated/test-options-adios/test-options-adios.cxx:3:

- #include "bout/options_io.hxx"
+ #include "bout/field3d.hxx"
+ #include "bout/options_io.hxx"


Options fields{{"f2d", Field2D(1.0)},
{"f3d", Field3D(2.0)},
{"fperp", FieldPerp(3.0)},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "FieldPerp" is directly included [misc-include-cleaner]

                   {"fperp", FieldPerp(3.0)},
                             ^

@ZedThree
Copy link
Copy Markdown
Member

Spotted some bugs in the adios implementation when adding some unit tests to this PR -- will merge this first, and make a new PR for tests + fixes.

@ZedThree ZedThree merged commit aae67a5 into next Nov 19, 2025
35 of 36 checks passed
@ZedThree ZedThree deleted the options-array-ints-rebase branch November 19, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants