Fix physical domain for benchmark stencils #1772
Conversation
ginkgo-bot
added
reg:testing
| * @param positions The position of this subdomain with respect to each | ||
| * dimension. | ||
| * @param target_local_size The desired size of the subdomains. The actual size | ||
| * can deviate from this to accommodate the square |
There was a problem hiding this comment.
nit-picking here, but shouldn't this be rectangular rather than square since we allow for different numbers of subdomains in each direction?
There was a problem hiding this comment.
The global domain is still square, so I reference that here.
There was a problem hiding this comment.
The documentation speaks of the square size of the subdomains though, which is a bit weird in itself now that I think about it, shouldn't it be square / rectangular shape? But this is really not important.
There was a problem hiding this comment.
I guess I wasn't clear, I changed it to now read the square size of the global domain, which I hope is better to understand.
benchmark/utils/stencil_matrix.hpp
Outdated
| return is_in_box(i, discretization_points) | ||
| auto target_position = [&](const IndexType dim, const IndexType i, | ||
| const int position) { | ||
| return is_in_box(i, discretization_points[dim]) |
There was a problem hiding this comment.
Maybe, since box is replaced with subdomain everywhere else, we could call this is_in_subdomain?
There was a problem hiding this comment.
I replaced it with range. I would consider the box here as something different from the subdomain, since the box is just the interval [0, n), but I thought that is_in_interval would be too long. So I'm now using is_in_range.
There was a problem hiding this comment.
I'm not 100% sure what the additional size parameter of the generated matrices is doing. All I know is that it can be zeros, and other times, it is the local_size. I think some clarification would be helpful.
Additionally, the return value documentation of the generation functions needs to be updated.
| * @param positions The position of this subdomain with respect to each | ||
| * dimension. | ||
| * @param target_local_size The desired size of the subdomains. The actual size | ||
| * can deviate from this to accommodate the square |
There was a problem hiding this comment.
The documentation speaks of the square size of the subdomains though, which is a bit weird in itself now that I think about it, shouldn't it be square / rectangular shape? But this is really not important.
benchmark/utils/general.hpp
Outdated
| template <typename VectorType> | ||
| std::unique_ptr<VectorType> create_manufactured_rhs( | ||
| std::shared_ptr<const gko::Executor> exec, const gko::LinOp* system_matrix, | ||
| const VectorType* solution, bool normalize) |
There was a problem hiding this comment.
No, I will rename this function to create_normalized_manufactured_rhs.
MarcelKoch
force-pushed
the
stencil-rescaling
branch
from
5076e82 to
cc3b0b4
Compare
MarcelKoch
force-pushed
the
stencil-rescaling
branch
4 times, most recently
from
a491211 to
0e45798
Compare
This implies that the process subdomains will not be square/cube shaped anymore. Instead, they can have arbitrary shape `d_x * d_y (* d_z)`. The values `d_x, d_y, d_z` are process dependent, but they differ by at most 1 each.
Signed-off-by: Marcel Koch <[email protected]>
- add documentation on empty dim - fix documentation of return type - rename `is_in_box` -> `is_in_range` - rename manufactured rhs Co-authored-by: Fritz Goebel <[email protected]> Co-authored-by: Thomas Grützmacher <[email protected]> Co-authored-by: Tobias Ribizel <[email protected]>
MarcelKoch
force-pushed
the
stencil-rescaling
branch
from
0e45798 to
3cc00bc
Compare
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1772 +/- ##
===========================================
- Coverage 89.31% 89.31% -0.01%
===========================================
Files 811 811
Lines 67506 67514 +8
===========================================
+ Hits 60296 60301 +5
- Misses 7210 7213 +3 ☔ View full report in Codecov by Sentry. |
This PR changes the stencil generation for the benchmarks to always use the fixed physical domain$[0, 1]^d$ . Before, the domain was scaling with the number of processes, as each process subdomain had to be a square/cube. Now, the shape of the subdomains (as in their number of degrees of freedom per dimension) is not square/cubic anymore, and can differ (slightly) between processes.
The subdomain sizes are chosen such that:
Example: Consider a 2D equidistant grid with 81 DOFs in each direction. Given 2x2 processes, then the process at
[0, 0]will have41 x 41DOFs, and the process[1, 0]will have40 x 41DOFs.As a consequence of the variable subdomain size, the computed process local sizes have to be propagated up from the stencil creation.