Conversation
Add BC-aware constructor overloads accepting Robin coefficient vectors dc (a0) and nc (b0). An axis is periodic when all dc/nc entries are zero, otherwise the existing non-periodic operator is used. Mixed periodicity supported in 2D/3D via per-axis checks. Adds two private static helpers: periodicGrad1D and isPeriodic. All original constructors unchanged.
Replace file(GLOB) with explicit source lists in src/cpp, tests/cpp, and examples/cpp CMakeLists.txt to avoid stale builds when files are added or renamed. Extend gradient.cpp with periodic boundary condition support: - Implement isPeriodic helper returning int instead of bool for efficiency - Remove Armadillo dependency in isPeriodic and zero-fill logic - Update dc and nc parameter types accordingly Update tests/cpp/test2.cpp to cover the periodic gradient case.
This reverts commit 5439812.
Add BC-aware constructor overloads accepting Robin coefficient vectors (dc/nc) for 1D, 2D, and 3D. Implement periodicGrad1D as a circulant sparse matrix via finite-difference stencil, with isPeriodic helper to detect periodic axes. Support mixed periodicity in 2D/3D through per-axis checking and Kronecker product assembly. Preserve backward compatibility by leaving original constructors untouched.
jbrzensk
left a comment
jbrzensk
left a comment
There was a problem hiding this comment.
Hi ya'll, thanks for this.
I tried it and it works appropriately. Most of the notes are on formatting. The clang-formatter flags almost every line, for spacing issues. It doesn't reject the PR but does generate a lot of errors that need to be addressed.
| // A | ||
| at(0, 0) = -8.0 / 3.0; | ||
| at(0, 1) = 3.0; | ||
| at(0, 1) = 3.0; |
There was a problem hiding this comment.
All the spaces and non-spaces are flagged by the linter.
Click lint-new-code, run super linter
| at(m - 1, m - 2) = 3.0 / 40.0; | ||
| at(m - 1, m - 3) = -1.0 / 168.0; | ||
| // Middle | ||
| at(0, 0) = -352.0/105.0; at(0, 1) = 35.0/8.0; |
There was a problem hiding this comment.
Same here. One line for each command, spaces between operators, no spaces after ;
| // first and last rows removed, leaving only the s interior rows. | ||
| static sp_mat trimmedIdentity(u32 s) { |
There was a problem hiding this comment.
Will this be used anywhere else? Maybe it should be in the utils.h?
There was a problem hiding this comment.
I agree, let me look into this. Thanks
There was a problem hiding this comment.
the trimmedIdentity function is currently used only for trimming in the gradient.cpp file, a similar operation is perform in the divergence.cpp but column-wise. The function was added to simply reduce code duplication.
A closer look, there are other sections of the divergence.cpp file that can also call this functions (with different arguments) and avoid duplications. For instance,
lines 242-245, and lines 268-273 of divergence.cpp.
@jbrzensk do you think that this would be useful to users as utility? or should it be kept internally?
There was a problem hiding this comment.
Its fine internal here as a static if nothing else could use it.
|
FYI, this work is being incrementally committed to allow for other team members to provide feedback as the work progresses. Thus, this Draft PR will have intermediate steps before becoming a PR. Other team members can incrementally review the C++ implementations of the three operators; gradient, divergence and laplacian in the feature/CPP_Parity_Ops branch. Here's a recap of the whole process:
|
3 participants
What type of PR is this? (check all applicable)
Description
Currently the mimetic operators in MATLAB/Octave work for periodic and non-periodic boundary conditions. The C++ implementation needs to include functionality to support the periodic cases. The new MATLAB/Octave interface returns mimetic operators depending on whether or not the operator contain a periodic boundary condition type a0 U + b0 dU/dn = g. However, the current C++ gradient constructors do not check for these boundary conditions, nor has an implementation for periodic boundaries. We propose creating new C++ constructors that match the grad implementations in MATLAB/Octave.
Related Issues & Documents
QA Instructions, Screenshots, Recordings
Run test2.cpp to test this code.
Added/updated tests?
have not been included
Read Contributing Guide and Code of Conduct
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?