Hermes-3 merges #1

mikekryjak · 2024-07-25T08:57:37Z

This is a Hermes-3 BOUT-dev merge to help with debugging a spurious slowdown in the nonlinear solves due to new, improved neutral transport equation flux limiter approach. The Hermes-3 PR for this work is here: mikekryjak/hermes-3#8

Merge of recent Next as of July 2024 with the CVODE modification to know when we're in a linear solve or not. This comes from this PR: boutproject#2932

And has further merges as per this latest commit: d7f88073f3ad707d624f8b9fed0a97a49596c542

I am also merging this with:

David's PVODE diagnostics, dumping debug files containing per-equation residuals on solver failure: Pvcode + Cvode improvements boutproject/BOUT-dev#2889 with commit boutproject@2875347
Steven's extra flag for CVODE allowing limiting nonlinear solver fails, allowing triggering failure after a specific number of nonlinear failures, which may allow us to engineer a failure triggered by the nonlinear solver struggling. Steven-Roberts@87e6877. The option is documented here: https://sundials.readthedocs.io/en/latest/cvode/Usage/index.html#c.CVodeSetMaxConvFails
David's implementation of the debug dumping for CVODE on this branch https://github.com/boutproject/BOUT-dev/tree/cvode-for-mike with this commit boutproject@acdc86c

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

Use the new track feature (better name required) to dump the different components of the ddt() as well as the residuum for the evolved fields.

This keeps track of all the changes done to the field and stores them to a OptionsObject.

gcc 9.4 is unable to correctly parse the construction for the function argument, if name.change is used directly. First making a copy seems to work around that issue.

…into hermes3

github-actions

clang-tidy made some suggestions

github-actions · 2024-07-25T09:03:40Z

src/mesh/data/gridfromfile.cxx

+  }
+
+  // Ensure that output variable has the correct size
+  var.resize(len);


warning: variable name 'it' is too short, expected at least 3 characters [readability-identifier-length]

const auto* it = std::begin(full_var); ^

github-actions · 2024-07-25T09:03:40Z

src/mesh/impls/bout/boutmesh.cxx

+  int limiter_count = 0;
+  Mesh::get(limiter_count, "limiter_count", 0);
+  if (limiter_count > 0) {
+    std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;


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;

github-actions · 2024-07-25T09:03:41Z

src/mesh/impls/bout/boutmesh.cxx

+    if (!source->get(this, limiter_xends, "limiter_xends", limiter_count)) {
+      throw BoutException("Couldn't read limiter_xend vector of length {} from mesh",
+                          limiter_count);
+    }


warning: variable 'yind' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

}

int const yind = limiter_yinds[i];

github-actions · 2024-07-25T09:03:41Z

src/mesh/impls/bout/boutmesh.cxx

+      throw BoutException("Couldn't read limiter_xend vector of length {} from mesh",
+                          limiter_count);
+    }
+


warning: variable 'xstart' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int const xstart = limiter_xstarts[i];

github-actions · 2024-07-25T09:03:41Z

src/mesh/impls/bout/boutmesh.cxx

+                          limiter_count);
+    }
+
+    for (int i = 0; i < limiter_count; ++i) {


warning: variable 'xend' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

for (int i = 0; i < limiter_count; ++i) {

int const xend = limiter_xends[i];

…to hermes3

mikekryjak

This one is just a merge of the automatic clang-format commits that desynced my local branch from the remote one.

github-actions

clang-tidy made some suggestions

github-actions · 2024-07-25T11:57:08Z

src/field/field3d.cxx

+template Options* Field3D::track<FieldPerp>(const FieldPerp&, std::string);
+
+Options* Field3D::track(const BoutReal& change, std::string operation) {
+  if (tracking and tracking_state) {


warning: implicit conversion 'Options *' -> bool [readability-implicit-bool-conversion]

Suggested change

if (tracking and tracking_state) {

if ((tracking != nullptr) and tracking_state) {

github-actions · 2024-07-25T11:57:08Z

src/field/field3d.cxx

+template Options* Field3D::track<FieldPerp>(const FieldPerp&, std::string);
+
+Options* Field3D::track(const BoutReal& change, std::string operation) {
+  if (tracking and tracking_state) {


warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion]

Suggested change

if (tracking and tracking_state) {

if (tracking and (tracking_state != 0)) {

github-actions · 2024-07-25T11:57:08Z

src/mesh/data/gridfromfile.cxx

+  // Ensure that output variable has the correct size
+  var.resize(len);
+
+  const auto* it = std::begin(full_var);


warning: variable name 'it' is too short, expected at least 3 characters [readability-identifier-length]

const auto* it = std::begin(full_var); ^

github-actions · 2024-07-25T11:57:08Z

src/mesh/impls/bout/boutmesh.cxx

+    }
+
+    for (int i = 0; i < limiter_count; ++i) {
+      int yind = limiter_yinds[i];


warning: variable 'yind' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int yind = limiter_yinds[i];

int const yind = limiter_yinds[i];

github-actions · 2024-07-25T11:57:08Z

src/mesh/impls/bout/boutmesh.cxx

+
+    for (int i = 0; i < limiter_count; ++i) {
+      int yind = limiter_yinds[i];
+      int xstart = limiter_xstarts[i];


warning: variable 'xstart' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int xstart = limiter_xstarts[i];

int const xstart = limiter_xstarts[i];

github-actions · 2024-07-25T11:57:10Z

src/solver/impls/pvode/pvode.cxx

+          .withDefault(false);
+
+  cvode_mem = CVodeMalloc(neq, solver_f, simtime, u, use_adam ? ADAMS : BDF, NEWTON, SS,
+                          &reltol, &abstol, this, nullptr, optIn, iopt, ropt, machEnv);


warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

&reltol, &abstol, this, nullptr, optIn, iopt, ropt, machEnv); ^

github-actions · 2024-07-25T11:57:10Z

src/solver/impls/pvode/pvode.cxx

@@ -293,6 +358,54 @@ BoutReal PvodeSolver::run(BoutReal tout) {
  // Check return flag
  if (flag != SUCCESS) {
    output_error.write("ERROR CVODE step failed, flag = {:d}\n", flag);
+    CVodeMemRec* cv_mem = (CVodeMem)cvode_mem;


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

CVodeMemRec* cv_mem = (CVodeMem)cvode_mem; ^

github-actions · 2024-07-25T11:57:10Z

src/solver/impls/pvode/pvode.cxx

+        for (const auto& prefix : {"pre_"s, "residuum_"s}) {
+          std::vector<Field3D> list_of_fields{};
+          std::vector<bool> evolve_bndrys{};
+          for (const auto& f : f3d) {


warning: variable name 'f' is too short, expected at least 3 characters [readability-identifier-length]

for (const auto& f : f3d) { ^

github-actions · 2024-07-25T11:57:10Z

src/solver/impls/pvode/pvode.cxx

+                              prefix == "pre_"s ? udata : N_VDATA(cv_mem->cv_acor));
+        }
+
+        for (auto& f : f3d) {


warning: variable name 'f' is too short, expected at least 3 characters [readability-identifier-length]

for (auto& f : f3d) { ^

github-actions · 2024-07-25T11:57:10Z

src/solver/impls/pvode/pvode.cxx

+        }
+        run_rhs(simtime);
+
+        for (auto& f : f3d) {


warning: variable name 'f' is too short, expected at least 3 characters [readability-identifier-length]

for (auto& f : f3d) { ^

bendudson and others added 30 commits June 14, 2023 12:06

Add Array<int> to Options

9d7f620

Enable 1D arrays of integers to be read from NetCDF files, and made available via `Mesh::get()`. Mainly useful for mesh construction.

GridFile: Check sizes, allocate vector ints

c8e3dd0

When reading 1D vector of ints, check that the input is long enough, and resize the output vector.

Merge branch 'bugfix-fvops-yalign' into hermes3-temp

1c77e00

Merge branch 'next' into hermes3-temp

7d261d4

Apply black changes

30a93f7

Dump field before and after rhs evaluation for debugging

fee27c9

Add setName function

96da2e9

Set div_par and grad_par names

e56981c

Dump debug file if PVODE fails

6b2c132

Use the new track feature (better name required) to dump the different components of the ddt() as well as the residuum for the evolved fields.

Add tracking to Field3D

df2d661

This keeps track of all the changes done to the field and stores them to a OptionsObject.

Add tracking to Field3D

263f9fe

This keeps track of all the changes done to the field and stores them to a OptionsObject.

cvode: Add option to use Adams Moulton solver instead of BDF

bac4ca9

Expose more pvode option to user

708bdcb

Fix bad cherry-pick

d88b454

Update to new API

023bc41

Fix documentation

affc995

Apply clang-format changes

71f5b6a

Apply recomendations from code-review

31fd461

Use more meaningful names

17e46cf

Apply clang-format changes

9c0ae16

Apply suggestions from code review

4a17b49

Workaround for gcc 9.4

2f7c3c0

gcc 9.4 is unable to correctly parse the construction for the function argument, if name.change is used directly. First making a copy seems to work around that issue.

Add option to debug on failure

d611758

Add option to euler solver to dump debug info

db96b7e

Disable tracking once we are done

84bfcef

Allow to dump every timestep with euler

73265df

Apply clang-format changes

8ff388a

Merge branch 'next' into hermes3-temp

d7f8807

Add debug_on_failure for cvode

acdc86c

Steven-Roberts and others added 3 commits July 24, 2024 10:50

Add cvode_max_conv_fails option

87e6877

Apply clang-format changes

ffca58c

Merge remote-tracking branch 'boutproject/pvcode-cvode-improvements' …

bb7c0f2

…into hermes3

github-actions bot reviewed Jul 25, 2024

View reviewed changes

mikekryjak added 2 commits July 25, 2024 10:17

Merge remote-tracking branch 'Steven-Roberts/cvode_max_conv_fails' in…

01951e8

…to hermes3

Merge remote-tracking branch 'boutproject/cvode-for-mike' into hermes3

44022f9

mikekryjak mentioned this pull request Jul 25, 2024

Redoing AFN neutral limiters mikekryjak/hermes-3#8

Open

mikekryjak and others added 2 commits July 25, 2024 12:47

merge

9cccd66

Apply clang-format changes

79821e2

mikekryjak commented Jul 25, 2024

View reviewed changes

github-actions bot reviewed Jul 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hermes-3 merges #1

Hermes-3 merges #1

mikekryjak commented Jul 25, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

mikekryjak left a comment

github-actions bot left a comment

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

github-actions bot Jul 25, 2024

-    std::vector<int> limiter_yinds, limiter_xstarts, limiter_xends;
+    std::vector<int> limiter_yinds;
+    std::vector<int> limiter_xstarts;
+    std::vector<int> limiter_xends;

	for (int i = 0; i < limiter_count; ++i) {
	int const xend = limiter_xends[i];

	if (tracking and tracking_state) {
	if ((tracking != nullptr) and tracking_state) {

	if (tracking and tracking_state) {
	if (tracking and (tracking_state != 0)) {

	int yind = limiter_yinds[i];
	int const yind = limiter_yinds[i];

	int xstart = limiter_xstarts[i];
	int const xstart = limiter_xstarts[i];

Hermes-3 merges #1

Are you sure you want to change the base?

Hermes-3 merges #1

Conversation

mikekryjak commented Jul 25, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

mikekryjak left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot Jul 25, 2024

Choose a reason for hiding this comment

mikekryjak commented Jul 25, 2024 •

edited

Loading