Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge v5 into next - resolve conflicts #2658

Merged
merged 20 commits into from
Feb 17, 2023
Merged

Merge v5 into next - resolve conflicts #2658

merged 20 commits into from
Feb 17, 2023

Conversation

dschwoerer
Copy link
Contributor

Merges next into master to resolve conflicts, so that #2655 can be merged

bendudson and others added 20 commits August 18, 2022 17:58
Solves `A + Div_par( B Grad_par )` in divergence form. Adapted from
`InvertPar` which uses `Grad2_par2 = Grad_par(Grad_par)` in
non-conservative form.
Enables the fluxes at the boundaries to be fixes to be the same as
the Spitzer-Harm calculation.
The integral of Div(Q) over the domain should be the same for SNB and
S-H methods, because the Neumann boundary conditions on H should
ensure this.
Runs in open domain with boundaries, checking that the boundary
fluxes are the same for SH and SNB calculations even with non-uniform mesh.
Strangely, changing calculation of dy, J from float to double makes
difference between SNB and SH heat fluxes larger.
Forgot to add, so wouldn't be compiled in Autotools build
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SNB and InvertParDiv for nonuniform grids
Copy link
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

*
* Field3D result = inv->solve(rhs);
*/
class InvertParDiv {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: class 'InvertParDiv' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class InvertParDiv {
      ^

}

protected:
CELL_LOC location;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: member variable 'location' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

  CELL_LOC location;
           ^


protected:
CELL_LOC location;
Mesh* localmesh; ///< Mesh object for this solver
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: member variable 'localmesh' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

  Mesh* localmesh; ///< Mesh object for this solver
        ^

nsys = 1 + (localmesh->LocalNz) / 2;
}

Field3D InvertParDivCR::solve(const Field3D& f) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function 'solve' has cognitive complexity of 88 (threshold 25) [readability-function-cognitive-complexity]

Field3D InvertParDivCR::solve(const Field3D& f) {
                        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:63: +1, including nesting penalty of 0, nesting level increased to 1

  ASSERT1(localmesh == f.getMesh());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:64: +1, including nesting penalty of 0, nesting level increased to 1

  ASSERT1(location == f.getLocation());
  ^

include/bout/assert.hxx:39: expanded from macro 'ASSERT1'

  if (!(condition)) {                                                                    \
  ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:78: +1, including nesting penalty of 0, nesting level increased to 1

  for (surf.first(); !surf.isDone(); surf.next()) {
  ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:80: +2, including nesting penalty of 1, nesting level increased to 2

    if (!surf.closed()) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:82: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.firstY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:83: +4, including nesting penalty of 3, nesting level increased to 4

        if (location == CELL_YLOW) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:86: +1, nesting level increased to 4

        } else {
          ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:90: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.lastY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:94: +3, including nesting penalty of 2, nesting level increased to 3

      if (n > size) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:113: +1, including nesting penalty of 0, nesting level increased to 1

  for (surf.first(); !surf.isDone(); surf.next()) {
  ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:124: +2, including nesting penalty of 1, nesting level increased to 2

    if (!closed) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:125: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.firstY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:126: +4, including nesting penalty of 3, nesting level increased to 4

        if (location == CELL_YLOW) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:131: +1, nesting level increased to 4

        } else {
          ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:136: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.lastY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:146: +2, including nesting penalty of 1, nesting level increased to 2

    for (int y = 0; y < localmesh->LocalNy - localmesh->ystart - local_ystart; y++) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:151: +2, including nesting penalty of 1, nesting level increased to 2

    for (int k = 0; k < nsys; k++) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:152: +3, including nesting penalty of 2, nesting level increased to 3

      for (int y = 0; y < localmesh->LocalNy - localmesh->ystart - local_ystart; y++) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:155: nesting level increased to 4

        auto Left = [&](const Field2D& field) {
                    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:158: nesting level increased to 4

        auto Right = [&](const Field2D& field) {
                     ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:182: +2, including nesting penalty of 1, nesting level increased to 2

    if (closed) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:189: +3, including nesting penalty of 2, nesting level increased to 3

      if (rank == 0) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:190: +4, including nesting penalty of 3, nesting level increased to 4

        for (int k = 0; k < nsys; k++) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:196: +3, including nesting penalty of 2, nesting level increased to 3

      if (rank == np - 1) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:197: +4, including nesting penalty of 3, nesting level increased to 4

        for (int k = 0; k < nsys; k++) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:203: +1, nesting level increased to 2

    } else {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:205: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.firstY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:206: +4, including nesting penalty of 3, nesting level increased to 4

        for (int k = 0; k < nsys; k++) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:207: +5, including nesting penalty of 4, nesting level increased to 5

          for (int y = 0; y < localmesh->ystart; y++) {
          ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:216: +3, including nesting penalty of 2, nesting level increased to 3

      if (surf.lastY()) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:217: +4, including nesting penalty of 3, nesting level increased to 4

        for (int k = 0; k < nsys; k++) {
        ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:218: +5, including nesting penalty of 4, nesting level increased to 5

          for (int y = size - localmesh->ystart; y < size; y++) {
          ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:234: +2, including nesting penalty of 1, nesting level increased to 2

    for (int k = 0; k < nsys; k++) {
    ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:235: +3, including nesting penalty of 2, nesting level increased to 3

      for (int y = 0; y < size; y++) {
      ^

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:241: +2, including nesting penalty of 1, nesting level increased to 2

    for (int y = 0; y < size; y++) {
    ^

int x = surf.xpos;

// Test if open or closed field-lines
BoutReal ts;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'ts' is not initialized [cppcoreguidelines-init-variables]

src/invert/pardiv/impls/cyclic/pardiv_cyclic.cxx:42:

- #include <bout/constants.hxx>
+ #include <math.h>
+ 
+ #include <bout/constants.hxx>
Suggested change
BoutReal ts;
BoutReal ts = NAN;


if (closed) {
// Twist-shift
int rank;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'rank' is not initialized [cppcoreguidelines-init-variables]

Suggested change
int rank;
int rank = 0;

if (closed) {
// Twist-shift
int rank;
int np;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'np' is not initialized [cppcoreguidelines-init-variables]

Suggested change
int np;
int np = 0;

};

namespace {
RegisterInvertParDiv<InvertParDivCR> registerinvertpardivcyclic{PARDIVCYCLIC};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'registerinvertpardivcyclic' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

RegisterInvertParDiv<InvertParDivCR> registerinvertpardivcyclic{PARDIVCYCLIC};
                                     ^

};

namespace {
RegisterInvertParDiv<InvertParDivCR> registerinvertpardivcyclic{PARDIVCYCLIC};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'registerinvertpardivcyclic' defined in a header file; variable definitions in header files can lead to ODR violations [misc-definitions-in-headers]

RegisterInvertParDiv<InvertParDivCR> registerinvertpardivcyclic{PARDIVCYCLIC};
                                     ^

@@ -19,6 +20,16 @@
throw BoutException("Line " S__LINE__ " Expected false, got true: " #expr); \
}

#define EXPECT_LT(expr1, expr2) \
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function-like macro 'EXPECT_LT' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define EXPECT_LT(expr1, expr2)                                                         \
        ^

Copy link
Contributor

@bendudson bendudson left a comment

Choose a reason for hiding this comment

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

Thanks @dschwoerer !

@bendudson bendudson merged commit 6d19d7e into master Feb 17, 2023
@bendudson bendudson deleted the master-next-merge-v5 branch February 17, 2023 19:13
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.

None yet

3 participants