1888/apply by MarcelKoch · Pull Request #2070 · ginkgo-project/ginkgo

MarcelKoch · 2026-08-17T16:46:30Z

This PR changes the apply interface to only allow applying linops to multivectors. The apply implementations are changed to use the precision dispatch built into the multi vector interface.

removed functionality: - implicit Diag * Csr - implicit Csr * Csr

Keep the header for alpha release and give a compiler error if used. The header will be removed in the full release.

This is unncessary, since it can be recreated by using the run function and the existing create functions.

yhmtsai · 2026-08-19T13:09:17Z

    virtual bool apply_uses_initial_guess() const { return false; }

-    [[nodiscard]] precision get_precision() const noexcept;
+    [[nodiscard]] precision get_precision() const noexcept { return value_t_; }


at some point we might need to split to two core library: one is the utils which can be used by the backend and the other composes the backend together.

yhmtsai · 2026-08-19T13:10:49Z

     * Implementers of LinOp should override this function instead
     * of apply(const LinOp *, const LinOp *, const LinOp *, LinOp *).


adding some note for the default implementation without mixed precision support

yhmtsai · 2026-08-19T13:12:49Z

+    std::visit(
+        [&](auto p) {
+            using value_type = std::decay_t<decltype(p)>;
+            auto dense_alpha = alpha->as_precision(this);
+            auto dense_beta = beta->as_precision(this);
+            auto x_clone = converted_x->clone();
+            this->apply_impl(converted_b.get(), x_clone.get());
+            converted_x->scale(dense_beta.get());
+            converted_x->add_scaled(dense_alpha.get(), x_clone);
+        },
+        precision_to_variant(this->get_precision()));


Suggested change

std::visit(

[&](auto p) {

using value_type = std::decay_t<decltype(p)>;

auto dense_alpha = alpha->as_precision(this);

auto dense_beta = beta->as_precision(this);

auto x_clone = converted_x->clone();

this->apply_impl(converted_b.get(), x_clone.get());

converted_x->scale(dense_beta.get());

converted_x->add_scaled(dense_alpha.get(), x_clone);

},

precision_to_variant(this->get_precision()));

auto dense_alpha = alpha->as_precision(this);

auto dense_beta = beta->as_precision(this);

auto x_clone = converted_x->clone();

this->apply_impl(converted_b.get(), x_clone.get());

converted_x->scale(dense_beta.get());

converted_x->add_scaled(dense_alpha.get(), x_clone);

value_type is not used, so you do not need the visit, right?

Yes, you're right. I will change this.

yhmtsai · 2026-08-19T13:14:30Z

    void write_data(json& output) { output["iterations"] = this->num_iters; }

+protected:
+    void on_iteration_complete(


this is considered public interface break, so we need to note it

I don't think we consider the benchmarks public interface. They are not even exported/installed.

yhmtsai · 2026-08-19T13:16:38Z

 }


-TEST_F(Dense, ApplyToMixedComplexIsEquivalentToRef)


these MixedComplex needs to be recovered?

No, they were deliberately removed, since this isn't (directly) supported anymore. We talked about this internally a while ago. The issue is essentially that this always requires two steps

convert complex to real (no precision conversion)

convert real to required precision

This creates two temporaries, which both need to be kept alive. I think it's best to leave it for the user to manage this. They can easily do it by themselves:

auto real = complex->create_real_view(); auto converted = real->as_precision(fp16)

Or switch the two steps.

IIRC, we start to use A(real) b (complex) is from IDR. Do we use it in other place.
Another option can be that we even do not support V x Complex.
Users can use A->apply(b->create_real_view(), x->create_real_view()) because create_real_view is here, so it will still valid when the internal temporary conversion get destroyed.

yhmtsai · 2026-08-19T13:32:29Z

+        max_relative_norm2 =
+            std::max(host_absolute_norm->at(0, i) / host_answer_norm->at(0, i),
+                     max_relative_norm2);


compute_norm will return multivector no matter it is distributed or not

Yes, all reductions (compute_dot/compute_norm) use gko::matrix::MultiVector for their result. This is also how it is currently. Maybe if we have a column distributed vector it would make sense to return something else, but for now it doesn't.

yhmtsai · 2026-08-19T13:59:35Z

-    scale_diag->apply(diag_mtx_, diag_mtx_);
-    scale_diag->apply(off_diag_mtx_, off_diag_mtx_);
+    auto diag_mtx_csr =
+        as<gko::matrix::Csr<ValueType, LocalIndexType>>(diag_mtx_);
+    auto off_diag_mtx_csr =
+        as<gko::matrix::Csr<ValueType, LocalIndexType>>(off_diag_mtx_);
+
+    scale_diag->apply(diag_mtx_csr, diag_mtx_csr);
+    scale_diag->apply(off_diag_mtx_csr, off_diag_mtx_csr);


I am thinking how to make generalized in the future when we support more scale on the matrix format.
Diagonal only scale csr and dense. here only deal with csr.
Dispatch here might be easy solution but it will not support user owned format, or we need to have feature class to allow that.

I think a feature class would be reasonable. But I think that is very far future right now, so I wouldn't worry too much.

yhmtsai · 2026-08-19T14:02:27Z

+    apply_precision_dispatch<ValueType>(
+        [this](auto dense_alpha, auto view_b, auto dense_beta, auto view_x,
+               auto...) {


what is auto... for?

In the mixed precision case it's used to pass through the actual value types of the arguments. But here it is not necessary, so I will remove it. That might also give better differentiation between the two dispatches.

MarcelKoch added this to the Ginkgo 2.0 milestone Aug 17, 2026

MarcelKoch self-assigned this Aug 17, 2026

MarcelKoch mentioned this pull request Aug 17, 2026

Tracking Issue: MultiVector #1888

Open

9 tasks

MarcelKoch force-pushed the 1888/derive-dist-vector branch from 1c82140 to ed710a9 Compare August 18, 2026 14:37

MarcelKoch force-pushed the 1888/apply branch 2 times, most recently from a4150cd to 89a9e63 Compare August 18, 2026 14:57

MarcelKoch force-pushed the 1888/derive-dist-vector branch from ed710a9 to 1dc8c10 Compare August 18, 2026 14:57

MarcelKoch added 1:ST:ready-for-review This PR is ready for review 1:ST:run-full-test labels Aug 18, 2026

MarcelKoch force-pushed the 1888/apply branch 2 times, most recently from 4f68eec to d2ab30c Compare August 19, 2026 10:22

MarcelKoch force-pushed the 1888/derive-dist-vector branch from 1dc8c10 to d0862a3 Compare August 19, 2026 12:38

MarcelKoch force-pushed the 1888/apply branch from d2ab30c to 6a34b65 Compare August 19, 2026 12:38

MarcelKoch added 5 commits August 19, 2026 14:56

change ScaledIdentityAddable to multivector

2aaee63

change linop::apply signature

2b2f685

deprecate apply using Dense

03ff0b8

add default implementation for advanced apply

400be05

change logger apply event

8cbb4ed

MarcelKoch added 16 commits August 19, 2026 14:56

add dispatch helper for apply functions

3c77add

fix hierarchy change for loggers

e4c652d

fix hierarchy change for stopping criteria

945f6c2

fix hierarchy change for matrices

ba293b1

removed functionality: - implicit Diag * Csr - implicit Csr * Csr

fix hierarchy change for base operators

d0dbe5a

fix hierarchy change for preconditioners

f3716b4

fix hierarchy change for reorder

6ea1c4c

fix hierarchy change for distributed

3d7794a

fix hierarchy change for solvers

bdc7166

fix hierarchy change for dense cache

63965d6

fix hierarchy change for multigrid level

37f546a

fix hierarchy change for benchmarks

36c8eed

fix hierarchy change for extensions

016e718

remove precision_dispatch implementation

104a66d

Keep the header for alpha release and give a compiler error if used. The header will be removed in the full release.

update tests

6448a51

remove temporary_conversion::create<T1, T2, ...> overload

f975caa

This is unncessary, since it can be recreated by using the run function and the existing create functions.

MarcelKoch force-pushed the 1888/apply branch from 6a34b65 to f975caa Compare August 19, 2026 12:56

MarcelKoch force-pushed the 1888/derive-dist-vector branch from d0862a3 to d22dff9 Compare August 19, 2026 12:56

yhmtsai reviewed Aug 19, 2026

View reviewed changes

		* Implementers of LinOp should override this function instead
		* of apply(const LinOp , const LinOp , const LinOp , LinOp ).

		}


		TEST_F(Dense, ApplyToMixedComplexIsEquivalentToRef)

Uh oh!

Conversation

MarcelKoch commented Aug 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants