1888/multivector - #2067
Conversation
ginkgo-bot
added
reg:build
d5dec4d to
a7ccae7
Compare
1dd91fc to
046c2c4
Compare
a7ccae7 to
9d3847a
Compare
046c2c4 to
2918d52
Compare
MarcelKoch
added
1:ST:ready-for-review
9d3847a to
c410e89
Compare
2918d52 to
b98f21d
Compare
Signed-off-by: Marcel Koch <marcel.koch@kit.edu>
b98f21d to
da07af9
Compare
c410e89 to
8445ef7
Compare
| */ | ||
| struct local_span : span { | ||
| using span::span; | ||
| struct local_span { |
There was a problem hiding this comment.
I am thinking the naming. span seems not to have special stuff for distributed. Could you remind me why we need to have two span type?
There was a problem hiding this comment.
The issue is that defining rows/cols for a submatrix is ambiguous in the distributed case. It could be either in local or global indexing. For now we only support local indexing. My idea was to make this clear by using this extra class. But maybe it is enough to name the arguments properly.
I would be fine with removing this if needed.
| } | ||
|
|
||
| namespace detail { | ||
|
|
| typename apply_to_list_impl<T, std::tuple<InputTypes...>, | ||
| std::tuple<ResultTypes..., T<U>>>::type; | ||
| }; | ||
|
|
| * @param p The requested precision | ||
| * @return A vector with the requested precision | ||
| */ | ||
| [[nodiscard]] temporary_conversion<AbstractMultiVector> as_precision( |
There was a problem hiding this comment.
there is also as_precision_without_iniitalized or as_precision_output to only prepare the precision without copy data from original, but it will copy back the data to original
There was a problem hiding this comment.
We only have make_temporary_output_clone which is similar to what you mentioned. I guess the two that you mention are better suited as create_with_precision or something similar.
| #define GKO_ASSERT_IS_DENSE(alpha) \ | ||
| { \ | ||
| bool is_dense = std::visit( \ | ||
| [alpha](auto p) { \ | ||
| using value_type = std::decay_t<decltype(p)>; \ | ||
| return dynamic_cast<const matrix::MultiVector<value_type>*>( \ | ||
| alpha.get()) != nullptr; \ | ||
| }, \ | ||
| precision_to_variant(alpha->get_precision())); \ | ||
| if (!is_dense) { \ | ||
| GKO_NOT_SUPPORTED(alpha); \ | ||
| } \ | ||
| } \ | ||
| static_assert(true, \ | ||
| "This assert is used to counter the false positive extra " \ | ||
| "semi-colon warnings") | ||
|
|
There was a problem hiding this comment.
if user define their own class inherited from AbstractMultiVector, it will not pass here.
There was a problem hiding this comment.
That is intended. The alpha arguments are only allowed to be gko::matrix::MultiVector.
| std::unique_ptr<const AbstractMultiVector> | ||
| AbstractMultiVector::create_real_view() const | ||
| { | ||
| return this->create_real_view_generic_impl(); |
There was a problem hiding this comment.
does generic here means implementation does not have the precision (compile time) information? same question to the other function
There was a problem hiding this comment.
Yes, it doesn't have the concrete (derived) type information here.
|
|
||
|
|
||
| template <template <typename...> typename T, typename TupleList> | ||
| using apply_to_list = typename detail::apply_to_list_impl<T, TupleList>::type; |
There was a problem hiding this comment.
it will give std::tuple<T...>, right?
There was a problem hiding this comment.
Yes. Although I don't remember why it was needed. Right now it looks like it is unused.
3 participants
This PR adds a generic interface for (multi-)vector types. The idea is to unify the handling of our
Denseanddistributed::Vectorclasses. Additionally, it could allow users more easily to provide their own types to be used in our solvers.Notes on the design:
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 usingstd::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)theDense<ValueType>needs to implement onlycompute_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
matrix::view::denseto the interface.The interface allows to temporary convert a vector into another precision using
as_precision. This works the same as the currentmake_temporary_conversion, but it is more flexible and simpler than that.