Simplify the handling of scalar parameters

[MarcelKoch]

Passing a scalar to some top-level functions can be quite bothersome. For example, the functions LinOp::apply(alpha, b, beta, x), Dense::scale(alpha), Dense::add_scaled(alpha, b) only accept a LinOp* as scalar. Also, passing a double would result in a compile-time error. Since this is quite unintuitive, a way to pass a single scalar into these functions should be introduced.

Current ideas:

1. add a new mixin EnableScalarApplyLinOp which implements overloads for LinOp::apply + add overloads to Dense
   • Pro:
     • simple to implement
   • Con:
     • need to add mixin to all matrix formats, solvers, preconditioners, etc
     • re-implementation of apply, which is already implemented twice
     • need to add overloads for new functions/mixin to new classes
     • does not work if only LinOp* is available
2. replace LinOp* parameter type by a variant-like type, e.g std::variant<LinOp*, double>
   • Pro:
     • no additions to exisiting code
     • only requires one definition for new functions
     • clearer distinction that the alpha parameter should not be any LinOp
   • Con:
     • requires implementation of std::variant (perhaps not all of that)
     • changes API, although user code should not break
     • unexpected typed (LinOp*, double, float,...) in a type union
3. introduce a function LinOp* gko::scalar(T) which creates the corresponding LinOp in a global cache
   • Pro:
     • no overloads/additions to exisiting code
   • Con:
     • handling global cache
     • additional function call by the user

I'm mostly in favor of 2., due to the noted pros. The second-best choice would be 1., and 3. is just for reference.

[MarcelKoch]

I think #179 and #37 are somewhat related to this issue.

[MarcelKoch]

This is some example code for the variant-like type in 2: https://godbolt.org/z/djqqjhesb

[upsj]

I prefer 1., since it produces the clearest interface without any indirection, since double and const LinOp* are not really alternative types I would expect inside a Sum Type. Since the implementation of the overloads can defer directly to the other overloads, this should not need a full re-implementation, but instead only act as a wrapper to the other EnableLinOp::* overloads. With this kind of thin and cheap-to-implement wrapper, I think the balance between development effort for us vs. usability improvement for applications heavily tilts in the applications' favor.

[tcojean]

As Tobias says, 1 should be relatively cheap to implement and manage, very much like some other mixins that we have. It makes code a bit less easy to understand, but considering the amount of mixins we have already, I don't think that is a main issue here.

[yhmtsai]

I also prefer 1, and 3 without global cache. Can 1 make EnableScalarApply inherited from EnableLinOp? it can take apply from EnableLinOp. (Is it what @upsj means for wrapper?) for 3, I would prefer std::shared_ptr scalar(value, exec) the specific short initialize for scalar. the usage is apply(scalar(1.0, exec).get(), ...)

[MarcelKoch]

For 1. if EnableScalarApply is implemented by inheriting from EnableLinOp, should that be a replacement of EnableLinOp in concrete classes or an addition. If it is an addition, would any problems appear due to diamond inheritance?

[upsj]

Yes, it should be a replacement (as a feature-extended specialization), otherwise we get diamond inheritance.

[MarcelKoch]

If it should be a replacement, are there any downsides to directly add the overloads to EnableLinOp? Or the other way around: are there any instances where the feature-extension is not wanted?

Of course, extending EnableLinOp directly would be interface-breaking, due to the required template parameter.

[upsj]

Yes, anything interface-breaking would only be in the scope of 2.0, and I think we can do this without obvious downsides without the need to change EnableLinOp directly. Some types like Permutation matrices don't have a ValueType.

[yhmtsai]

if just wrapping the another apply in EnableLinOp, it should be the same. and all needs to contain value_type in the class

[MarcelKoch]

Instead of introducing a new mixin, we could also add a specialization of EnableLinOp for the case if the ConcreteType contains the value_type member. We wouldn't need to find all the places where we need to replace the mixin, but we need to copy most of the EnableLinOp implementation (or introduce a base class?). Any thoughts on that?

[upsj]

@MarcelKoch I like the idea! I'm just not sure whether it is better to be explicit with a different type, or do this implicitly based on value_type, which might be hidden a bit too much, in case users want this functionality as well?