Dirichlet bcond by pdziekan · Pull Request #499 · igfuw/libmpdataxx

pdziekan · 2025-12-22T14:36:23Z

No description provided.

Copilot

Pull request overview

This pull request implements Dirichlet (fixed) boundary conditions for the libmpdata++ library, allowing variables to maintain constant values at domain boundaries based on their initial edge values. The implementation includes upgrading the C++ standard from C++14 to C++17 to leverage modern language features like if constexpr.

Added fixed boundary condition implementations for 2D and 3D cases
Integrated save_edge_val infrastructure across all existing boundary condition types
Added a test case demonstrating fixed boundary conditions in a 2D inflow scenario

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
tests/sandbox/inflow/inflow_2d.cpp	New test case for 2D inflow with fixed boundary condition on left edge
tests/sandbox/inflow/CMakeLists.txt	Build configuration for the inflow test
tests/sandbox/CMakeLists.txt	Added inflow subdirectory to test build
libmpdata++/bcond/fixed_2d.hpp	New 2D fixed (Dirichlet) boundary condition implementation
libmpdata++/bcond/fixed_3d.hpp	New 3D fixed (Dirichlet) boundary condition implementation
libmpdata++/bcond/detail/bcond_common.hpp	Added fixed enum value and save_edge_val virtual methods
libmpdata++/solvers/detail/solver_common.hpp	Added save_edges call during initialization
libmpdata++/solvers/detail/solver_2d.hpp	Implemented save_edges for all scalar variables in 2D
libmpdata++/solvers/detail/solver_3d.hpp	Implemented save_edges for all scalar variables in 3D
libmpdata++/bcond/cyclic_2d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/cyclic_3d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/open_2d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/open_3d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/rigid_2d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/rigid_3d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/remote_2d.hpp	Added empty save_edge_val stub implementation
libmpdata++/bcond/remote_3d.hpp	Added empty save_edge_val stub implementation
libmpdata++/concurr/detail/concurr_common.hpp	Added includes for fixed and gndsky boundary conditions
libmpdata++-config.cmake	Updated C++ standard from C++14 to C++17 and commented out C++14 compatibility check

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T14:41:47Z

libmpdata++/bcond/detail/bcond_common.hpp


+        virtual void save_edge_val(const arr_3d_t &, const rng_t &, const rng_t &)
+        {
+          assert(false && "bcond::save_edge_vel() called!");


Inconsistent error message in assertion. The assertion message says "bcond::save_edge_vel() called!" but this is actually the save_edge_val function. The error message should be "bcond::save_edge_val() called!" to accurately reflect which function was called.

Suggested change

assert(false && "bcond::save_edge_vel() called!");

assert(false && "bcond::save_edge_val() called!");

Copilot · 2025-12-22T14:41:47Z

libmpdata++/bcond/fixed_2d.hpp

+      void save_edge_val(const arr_t &a, const arr_t &val, const rng_t &j)
+      {
+        using namespace idxperm;
+        // assert(a.shape() == val.shape());


Commented-out assertion should be either enabled or removed. Line 48 contains a commented assertion to verify that array shapes match. If this check is needed, it should be enabled within an #if !defined(NDEBUG) block like the assertion on line 35. If the check is not needed, the commented code should be removed to reduce clutter and improve maintainability.

Copilot · 2025-12-22T14:41:47Z

libmpdata++/bcond/fixed_2d.hpp

+      void save_edge_val(const arr_t &a, const arr_t &val, const rng_t &j)
+      {
+        using namespace idxperm;
+        // assert(a.shape() == val.shape());


Commented-out assertion should be either enabled or removed. Line 88 contains a commented assertion to verify that array shapes match. If this check is needed, it should be enabled within an #if !defined(NDEBUG) block like the assertion in the left direction implementation. If the check is not needed, the commented code should be removed to reduce clutter and improve maintainability.

Copilot · 2025-12-22T14:41:48Z

libmpdata++/bcond/detail/bcond_common.hpp

          assert(false && "bcond::save_edge_vel() called!");
        };

+        virtual void save_edge_val(const arr_3d_t &, const rng_t &, const rng_t &)


Missing parameter name in virtual function signature. The second parameter "const arr_3d_t &" lacks a descriptive name, making the function signature less readable. While this is valid C++, adding a parameter name like "val" would improve code documentation and consistency with the actual implementations in fixed_3d.hpp.

Suggested change

virtual void save_edge_val(const arr_3d_t &, const rng_t &, const rng_t &)

virtual void save_edge_val(const arr_3d_t &val, const rng_t &i, const rng_t &j)

Copilot · 2025-12-22T14:41:48Z

libmpdata++/bcond/fixed_3d.hpp

+        // assert(a.shape() == val.shape());
+        auto s = a.shape();
+        s[d] = 1;
+        edge_values.emplace(&a, arr_t(s));


Potential memory leak or redundant allocation with emplace. The code uses emplace(&a, arr_t(s)) which always tries to insert a new element even if the key already exists. If save_edge_val is called multiple times with the same array pointer, emplace will fail to insert but the temporary arr_t(s) is still constructed. Consider using try_emplace or checking if the key exists before creating the array, or using insert_or_assign if re-initialization is intended.

Suggested change

edge_values.emplace(&a, arr_t(s));

edge_values.try_emplace(&a, s);

libmpdata++/bcond/fixed_3d.hpp

Copilot · 2025-12-22T14:41:51Z

libmpdata++-config.cmake

+## C++14
+#include(CheckCXXSourceCompiles)
+#set(CMAKE_REQUIRED_FLAGS "-std=c++14")
+#check_cxx_source_compiles("
+#  #include <type_traits>
+#  auto f() { return 1;}
+#  template <bool a, class b> using ei=std::enable_if<a,b>; 
+#  struct a {a(int){}};struct b:a {using a::a;};  
+#  int main(){b i(1);}
+#" CXX14_SUPPORTED)
+#if (NOT CXX14_SUPPORTED)
+#  message(FATAL_ERROR "C++14 compatibility test failed - please update your compiler or point CMake to another one with -DCMAKE_CXX_COMPILER=...")
+#endif()
+#unset(CMAKE_REQUIRED_FLAGS)


Commented-out code section should be removed. Lines 66-79 contain a commented-out C++14 compatibility check that is no longer needed since the codebase has been upgraded to C++17. Instead of commenting out this code, it should be completely removed to improve code maintainability and reduce clutter.

Suggested change

## C++14

#include(CheckCXXSourceCompiles)

#set(CMAKE_REQUIRED_FLAGS "-std=c++14")

#check_cxx_source_compiles("

# #include <type_traits>

# auto f() { return 1;}

# template <bool a, class b> using ei=std::enable_if<a,b>;

# struct a {a(int){}};struct b:a {using a::a;};

# int main(){b i(1);}

#" CXX14_SUPPORTED)

#if (NOT CXX14_SUPPORTED)

# message(FATAL_ERROR "C++14 compatibility test failed - please update your compiler or point CMake to another one with -DCMAKE_CXX_COMPILER=...")

#endif()

#unset(CMAKE_REQUIRED_FLAGS)

Copilot · 2025-12-22T14:41:51Z

libmpdata++/bcond/fixed_2d.hpp

+        // assert(a.shape() == val.shape());
+        auto s = a.shape();
+        s[d] = 1;
+        edge_values.emplace(&a, arr_t(s));


Potential memory leak or redundant allocation with emplace. The code uses emplace(&a, arr_t(s)) which always tries to insert a new element even if the key already exists. If save_edge_val is called multiple times with the same array pointer, emplace will fail to insert but the temporary arr_t(s) is still constructed. Consider using try_emplace or checking if the key exists before creating the array, or using insert_or_assign if re-initialization is intended.

Copilot · 2025-12-22T14:41:52Z

libmpdata++/bcond/detail/bcond_common.hpp


+        virtual void save_edge_val(const arr_2d_t &, const arr_2d_t &, const rng_t &)
+        {
+          assert(false && "bcond::save_edge_vel() called!");


Inconsistent error message in assertion. The assertion message says "bcond::save_edge_vel() called!" but this is actually the save_edge_val function. The error message should be "bcond::save_edge_val() called!" to accurately reflect which function was called.

Suggested change

assert(false && "bcond::save_edge_vel() called!");

assert(false && "bcond::save_edge_val() called!");

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

pdziekan · 2026-02-04T13:03:02Z

TODO: fixed bcond for sgs fields; right-hand fixed bcond doesnt work (?)

pdziekan added 2 commits December 22, 2025 13:34

inflow sandbox test

521b164

fixed boundary conditions

7e7ffc7

Copilot AI review requested due to automatic review settings December 22, 2025 14:36

Copilot started reviewing on behalf of pdziekan December 22, 2025 14:36 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

pdziekan and others added 5 commits January 8, 2026 13:21

Merge branch 'master' of github.com:igfuw/libmpdataxx into dirichlet

09fea5e

save_edge_val sharedmem bcond

dc7459d

Apply suggestions from code review

666ebf6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

merge

c1690c6

dirichlet fixes

bed3cdd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dirichlet bcond#499

Dirichlet bcond#499
pdziekan wants to merge 7 commits intoigfuw:masterfrom
pdziekan:dirichlet

pdziekan commented Dec 22, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

pdziekan commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	assert(false && "bcond::save_edge_vel() called!");
	assert(false && "bcond::save_edge_val() called!");

	virtual void save_edge_val(const arr_3d_t &, const rng_t &, const rng_t &)
	virtual void save_edge_val(const arr_3d_t &val, const rng_t &i, const rng_t &j)

	edge_values.emplace(&a, arr_t(s));
	edge_values.try_emplace(&a, s);

Conversation

pdziekan commented Dec 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

pdziekan commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant