Skip to content

WIP: Multivector interface #1889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 18 commits into
base: develop
Choose a base branch
from
Draft

WIP: Multivector interface #1889

wants to merge 18 commits into from

Conversation

MarcelKoch
Copy link
Member

@MarcelKoch MarcelKoch commented Jul 16, 2025

This PR adds a generic interface for (multi-)vector types. The idea is to unify the handling of our Dense and distributed::Vector classes. Additionally, it could allow users more easily to provide their own types to be used in our solvers.

Notes on the design:

The multivector interface inherits from LinOp, since removing the inheritance from LinOp for our vectors is a significant change that would leave Ginkgo broken for a very long time. However, I would prefer to separate these two interfaces. (This might be a long term change, not even for v2.0.)

The interface is completely generic; this especially means that it is not templated with a ValueType. One of the main issues regarding that is that the interface can't differentiate between complex and real vectors. I'm not sure if this is something we need to safeguard against at compile time, or if the checks at runtime will be sufficient. To remove the value type from places where our current interfaces rely on it (fill, scale, add_scaled, ...) I resorted to using std::variant. We always know the allowed value types (except for the complex/real issue), so I think it's reasonable to use a variant there.

I also provide a mixin for the interface, which concretizes most of the functions again for the known derived type. The actual implementations of the interface should derive from the mixin. If they do so, then they only need to implement concretized functions. E.g. instead of implementing compute_norm2_impl(MultiVector* result) the Dense<ValueType> needs to implement only compute_norm2_impl(Dense<ValueType>::absolute_type*).

I'm still in the process of figuring out how the precision dispatch can work. Right now I have the templated functions std::unique_ptr<MultiVector> temporary_precision<ValueType>(). I'm not sure if that is the approach we will pursue in the end.

Since we pass the (local) vector to our kernels, I needed to add some functionality to provide a std::unique_ptr<Dense<...>> to the interface. TBH, I don't like that this is a Dense and I would prefer if we could directly return a device view of Dense.

@MarcelKoch MarcelKoch self-assigned this Jul 16, 2025
@ginkgo-bot ginkgo-bot added reg:build This is related to the build system. mod:core This is related to the core module. type:matrix-format This is related to the Matrix formats labels Jul 16, 2025
@MarcelKoch MarcelKoch mentioned this pull request Jul 16, 2025
4 tasks
@MarcelKoch MarcelKoch requested a review from a team July 16, 2025 10:04
@ginkgo-bot
Copy link
Member

Error: The following files need to be formatted:

benchmark/blas/blas.cpp

You can find a formatting patch under Artifacts here or run format! if you have write access to Ginkgo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod:core This is related to the core module. reg:build This is related to the build system. type:matrix-format This is related to the Matrix formats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants