Adds Block operator

[MarcelKoch]

This PR adds a block operator to Ginkgo. Similar to Combination, and Composition, this linear operator is build up from other linear operator. It represents block matrices of the form:

$$Op = \begin{bmatrix} A & B \\\ C & D \end{bmatrix}$$

Individual blocks can be 0, by using a nullptr, but no row or column can consist only of nullptr. The block operator does not need to be square.

The block operator can be created in the following way:

std::shared_ptr<LinOp> A;
std::shared_ptr<LinOp> B;
std::shared_ptr<LinOp> C;
std::shared_ptr<LinOp> D;
auto bop = BlockOperator::create(exec, {{A, B}, {C, D}});

The passed-in LinOps are copied to the correct executor if necessary.

The block operator can be applied to Dense vectors, or block operators (currently only single block column).

Todo:

• documentation
• apply with >1 block column -> not necessary due to below
• maybe separate concerns and use block operator only for matrices and not for vectors -> only use BlockOperator for block matrices, not for vectors

[yhmtsai]

LGTM in general. Do you have some example or integration request this feature?

• No EnableCreateMethod already?
• multiple right hand side or multiple col block in right hand side
• empty row and empty col allowance? It should be okay when only apply but it may not make sense when using them in solver.

[MarcelKoch]

LGTM in general. Do you have some example or integration request this feature?

I did not intend to write an example for that. I think it has been mentioned somewhere before, but adding a new example for each new feature seems a bit overkill to me. Something like this could best be handled through documentation, but of course our current setup is not well suited for that.

* No EnableCreateMethod already?

Yes. I think that was a great suggestion by @upsj, so I'm using it, since it also has no effect on the user interface.

* multiple right hand side or multiple col block in right hand side

Since I'm not allowing using BlockOperator for block vectors, there should be no confusion here anymore. I decided to disallow block vectors because it requires non-const access to the blocks, and those block vectors could not be used in solvers anyway. So if we want to support block-vectors, that would have to be its own class.

* empty row and empty col allowance? It should be okay when only apply but it may not make sense when using them in solver.

It would be ok if the passed-in blocks are empty matrices (think of Csr::create(exec)). I only do not allow a row/column full of nullptrs, because that complicates determining the overall size of the block operator.

@yhmtsai

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
22 Code Smells

86.4% Coverage
0.0% Duplication

The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

[pratikvn]

Nice work! Mostly looks good to me. I think as this supports arbitrary block sizes, that should be tested as well. I think adding apply and advanced apply tests also for a block size of 3x3 would be good.

To ease comparisons, you can just create a single Dense matrix, create submatrices from it and compare the apply of both, separately.

[pratikvn]

Is there a need to unroll them ? Why not just have std::vector<std::vector<...>> instead of std::vector<...> ?

[MarcelKoch]

I think there is no real reason to prefer one over the other. I can't remember why I picked the flat vector, but at least it's a bit nicer for the assignment operators.

[pratikvn]

This is already an internal header, does it need to be in the detail namespace ?

[MarcelKoch]

We also use that namespace in our private files from time to time. Here I use it because the run_impl functions should not be used directly and are just an implementation detail.

[pratikvn]

Can maybe be formatted better:

Suggested change

	exec, {{gko::initialize<Mtx>({{1, 2}, {2, 1}}, exec),
	gko::initialize<Mtx>({{5, 6}, {6, 5}}, exec)},
	{nullptr, gko::initialize<Mtx>({{3, 4}, {4, 3}}, exec)}});
	exec, {
	{gko::initialize<Mtx>({{1, 2}, {2, 1}}, exec), gko::initialize<Mtx>({{5, 6}, {6, 5}}, exec)},
	{nullptr, gko::initialize<Mtx>({{3, 4}, {4, 3}}, exec)}
	});

[MarcelKoch]

clang-format says no. I would rather not put clang-format off in there, since I don't think the formatting is that bad, or that this really needs proper formatting.

[pratikvn]

Is it necessary to clone these block operators ? The apply should handle that correctly anyway. I think there is no need to have all the blocks on the same executor.

[MarcelKoch]

I'm following the same approach as Composition and Combination, which both do the same. I think it's better to be consistent here.

[sonarqubecloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

10 New issues
0 Security Hotspots
88.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[codecov]

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (2033f10) 89.35% compared to head (fb70ec0) 91.04%.

❗ Current head fb70ec0 differs from pull request most recent head d54f513. Consider uploading reports for the commit d54f513 to get more accurate results

Files	Patch %	Lines
core/base/block_operator.cpp	90.15%	13 Missing ⚠️
core/test/base/block_operator.cpp	97.60%	3 Missing ⚠️
core/base/dispatch_helper.hpp	87.50%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1435      +/-   ##
===========================================
+ Coverage    89.35%   91.04%   +1.69%     
===========================================
  Files          697      700       +3     
  Lines        56940    56996      +56     
===========================================
+ Hits         50880    51894    +1014     
+ Misses        6060     5102     -958     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ginkgo-bot]

Error: PR already merged!