Skip to content

Add factory for LaplaceXY, LaplaceXY2, LaplaceXY2Hypre#3187

Merged
bendudson merged 10 commits into
nextfrom
fix-laplacexy-warning
Oct 24, 2025
Merged

Add factory for LaplaceXY, LaplaceXY2, LaplaceXY2Hypre#3187
bendudson merged 10 commits into
nextfrom
fix-laplacexy-warning

Conversation

@ZedThree
Copy link
Copy Markdown
Member

Not quite sure why these were standalone classes, or if we indeed still want to keep all of them, but I've now squashed them into the generic factory capability so they can be switched between at runtime -- and we also avoid an annoying compiletime warning if compiling without PETSc or HYPRE!

@ZedThree ZedThree requested a review from bendudson October 23, 2025 17:19
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/invert/laplacexy.hxx
Comment thread include/bout/invert/laplacexy.hxx
Comment thread include/bout/invert/laplacexy.hxx
Comment thread include/bout/invert/laplacexy.hxx Outdated
Comment thread include/bout/invert/laplacexy.hxx
Comment thread src/invert/laplacexy/impls/petsc/laplacexy-petsc.hxx
Comment thread src/invert/laplacexy/impls/petsc/laplacexy-petsc.hxx
Comment thread tests/integrated/test-laplacexy-short/test-laplacexy.cxx
Comment thread tests/integrated/test-laplacexy-short/test-laplacexy.cxx
Comment thread tests/integrated/test-laplacexy-short/test-laplacexy.cxx
@ZedThree ZedThree force-pushed the fix-laplacexy-warning branch from 59a68f3 to fbe8a7f Compare October 24, 2025 17:10
@ZedThree
Copy link
Copy Markdown
Member Author

I think there's still more that could be tidied up here, but at least we can switch between PETSc and HYPRE at runtime, and there's few compile warnings.

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

#include <bout/bout.hxx>
#include <bout/constants.hxx>
#include <bout/derivs.hxx>
#include <bout/field_factory.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: duplicate include [readability-duplicate-include]

examples/elm-pb-outerloop/elm_pb_outerloop.cxx:34:

- #include <bout/derivs.hxx>
- #include <bout/derivs.hxx>
+ #include <bout/derivs.hxx>

#include <bout/invert/laplacexy.hxx>
#include <bout/invert_laplace.hxx>
#include <bout/invert_parderiv.hxx>
#include <bout/msg_stack.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: duplicate include [readability-duplicate-include]

examples/elm-pb-outerloop/elm_pb_outerloop.cxx:40:

- #include <bout/invert_laplace.hxx>
- #include <bout/invert_laplace.hxx>
+ #include <bout/invert_laplace.hxx>

BOUT_OVERRIDE_DEFAULT_OPTION("phi:bndry_xin", "none");
BOUT_OVERRIDE_DEFAULT_OPTION("phi:bndry_xout", "none");

#if BOUT_HAS_HYPRE
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 "BOUT_HAS_HYPRE" is directly included [misc-include-cleaner]

examples/elm-pb/elm_pb.cxx:7:

- #include <bout/bout.hxx>
+ #include "bout/build_defines.hxx"
+ #include <bout/bout.hxx>


/// Create a LaplaceXY object
LaplaceXY laplacexy(bout::globals::mesh);
auto laplacexy = LaplaceXY::create(bout::globals::mesh);
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 "bout::globals::mesh" is directly included [misc-include-cleaner]

examples/laplacexy/simple/test-laplacexy.cxx:1:

- #include <bout/bout.hxx>
+ #include "bout/globals.hxx"
+ #include <bout/bout.hxx>

Field2D x = 0.0;

x = laplacexy.solve(rhs, x);
Field2D result = laplacexy->solve(rhs, 0.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 'result' of type 'Field2D' can be declared 'const' [misc-const-correctness]

Suggested change
Field2D result = laplacexy->solve(rhs, 0.0);
Field2D const result = laplacexy->solve(rhs, 0.0);

// LaplaceXY object that this monitor belongs to
LaplaceXY& laplacexy;
};
static std::unique_ptr<LaplaceXY> create(Mesh* m = nullptr, Options* opt = nullptr,
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::unique_ptr" is directly included [misc-include-cleaner]

include/bout/invert/laplacexy.hxx:40:

- #include <string>
+ #include <memory>
+ #include <string>

@bendudson bendudson merged commit 1a13819 into next Oct 24, 2025
1 check passed
@bendudson bendudson deleted the fix-laplacexy-warning branch October 24, 2025 18:45
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.

2 participants