Avoid non-type template parameters

[Nils Deppe]

Hi Klaus,

This is a minor feature request. Blaze currently uses a lot of non-type template parameters, specifically for the alignment and padding flags. Template metaprogramming is much easier with only type template parameters so having the bool template parameters be replaced by std::integral_constant<bool, ...> would be nice. It would probably be easier to just declare a type alias

namespace blaze {
template <bool B>
using bool_ = std::integral_constant<bool, B>;
}  // namespace blaze

This change would be particularly useful with the type trait is_a:

template <template <typename...> class U, typename T>
struct is_a : std::false_type {};

template <template <typename...> class U, typename... Args>
struct is_a<U, U<Args...>> : std::true_type {};

I use this quite extensively and for this reason really avoid non-type template parameters if at all possible. Then, if one wants to check if something is a blaze::DenseVector one could just write is_a<blaze::DenseVector, T>::value. Thanks!

Best,

Nils

[Klaus Iglberger]

Hi Nils!

The Alias bool_ already exists, but is named Bool (see the header file <blaze/util/mpl/Bool.h>). You also don't have to write type traits for the Blaze types, but can use the ones provided by Blaze.

Utility type traits (blaze/util/typetraits):

AddCV             HaveSameSize      IsConvertible     IsPod             MakeUnsigned
AddConst          IsArithmetic      IsDestructible    IsPointer         Rank
AddPointer        IsArray           IsDouble          IsRValueReference RemoveAllExtents
AddReference      IsAssignable      IsEmpty           IsReference       RemoveCV
AddVolatile       IsBaseOf          IsEnum            IsSame            RemoveConst
AlignmentOf       IsBoolean         IsFloat           IsShort           RemoveExtent
All               IsBuiltin         IsFloatingPoint   IsSigned          RemovePointer
Any               IsCharacter       IsInteger         IsUnion           RemoveReference
CommonType        IsClass           IsIntegral        IsUnsigned        RemoveVolatile
Decay             IsComplex         IsLValueReference IsValid           TypeTraits
Extent            IsComplexDouble   IsLong            IsVectorizable    Void
GetMemberType     IsComplexFloat    IsLongDouble      IsVoid
HasMember         IsConst           IsNumeric         IsVolatile
HasSize           IsConstructible   IsObject          MakeSigned

Math type traits (blaze/math/typetraits):

Columns              HasSIMDLog2          IsDeclaration        IsResizable          IsUniUpper
HasAdd               HasSIMDMax           IsDenseMatrix        IsRestricted         IsUniform
HasConstDataAccess   HasSIMDMin           IsDenseVector        IsRow                IsUpper
HasDiv               HasSIMDMult          IsDiagonal           IsRowMajorMatrix     IsVecEvalExpr
HasMax               HasSIMDPow           IsDivExpr            IsRowVector          IsVecMapExpr
HasMin               HasSIMDRound         IsEvalExpr           IsSIMDCombinable     IsVecScalarDivExpr
HasMult              HasSIMDSin           IsExpression         IsSIMDEnabled        IsVecScalarMultExpr
HasMutableDataAccess HasSIMDSinh          IsGeneral            IsSIMDPack           IsVecSerialExpr
HasSIMDAbs           HasSIMDSqrt          IsHermitian          IsSMPAssignable      IsVecTVecMultExpr
HasSIMDAcos          HasSIMDSub           IsIdentity           IsSchurExpr          IsVecTransExpr
HasSIMDAcosh         HasSIMDTan           IsInvertible         IsSerialExpr         IsVecVecAddExpr
HasSIMDAdd           HasSIMDTanh          IsLower              IsShrinkable         IsVecVecDivExpr
HasSIMDAsin          HasSIMDTrunc         IsMatEvalExpr        IsSparseElement      IsVecVecMapExpr
HasSIMDAsinh         HasSub               IsMatInvExpr         IsSparseMatrix       IsVecVecMultExpr
HasSIMDAtan          HighType             IsMatMapExpr         IsSparseVector       IsVecVecSubExpr
HasSIMDAtanh         IsAdaptor            IsMatMatAddExpr      IsSquare             IsVector
HasSIMDCbrt          IsAddExpr            IsMatMatMapExpr      IsStatic             IsView
HasSIMDCeil          IsAligned            IsMatMatMultExpr     IsStrictlyLower      IsZero
HasSIMDConj          IsBLASCompatible     IsMatMatSubExpr      IsStrictlyTriangular LowType
HasSIMDCos           IsBinaryMapExpr      IsMatScalarDivExpr   IsStrictlyUpper      RemoveAdaptor
HasSIMDCosh          IsColumn             IsMatScalarMultExpr  IsSubExpr            RequiresEvaluation
HasSIMDDiv           IsColumnMajorMatrix  IsMatSerialExpr      IsSubmatrix          Rows
HasSIMDErf           IsColumnVector       IsMatTransExpr       IsSubvector          Size
HasSIMDErfc          IsComputation        IsMatVecMultExpr     IsSymmetric          StorageOrder
HasSIMDExp           IsCrossExpr          IsMatrix             IsTVecMatMultExpr    TransposeFlag
HasSIMDExp10         IsCustom             IsMultExpr           IsTemporary          TypeTraits
HasSIMDExp2          IsDeclDiagExpr       IsNumericMatrix      IsTransExpr          UnderlyingBuiltin
HasSIMDFloor         IsDeclExpr           IsNumericVector      IsTransformation     UnderlyingElement
HasSIMDInvCbrt       IsDeclHermExpr       IsOperation          IsTriangular         UnderlyingNumeric
HasSIMDInvSqrt       IsDeclLowExpr        IsOpposedView        IsUnaryMapExpr
HasSIMDLog           IsDeclSymExpr        IsPadded             IsUniLower
HasSIMDLog10         IsDeclUppExpr        IsProxy              IsUniTriangular

The one you are looking for is IsDenseVector in the <blaze/math/typetraits/IsDenseVector.h> header file. The type traits are unfortunately not documented in the wiki, but the complete Blaze documentation contains a separate section for type traits.

Does this serve your needs and fulfill the feature request?

Best regards,

Klaus!

[Nils Deppe]

Hi Klaus,

Sorry for the delay. This is a reasonable workaround. Would you be open to pull request if I find it inconvenient to use all the extra type traits? is_a as I showed above really cuts down on how many type traits are necessary. I don't want to spend the time making this magnitude of change if it cannot be pushed upstream.

Best,

Nils

[Klaus Iglberger]

Hi Nils!

You might underestimate the real magnitude of this change: You would have to rework every major class in Blaze, would have to adapt many files since type traits are used everywhere, and would have to rethink many places where the logic changes since the proposed type trait is not exactly the same as the existing type trait. Also, the interface of every major class would change since now it would be no longer possible to pass boolean values (which strictly speaking would require us to increment the major version number).

No, we would not accept a pull request of this magnitude and of this impact. Showing all the available type traits was our way of saying: "All the functionality is already in place, we don't have to invest this enormous amount of time." Although we're grateful for any proposal to improve Blaze, we believe that the 'inconvenient" argument is not strong enough in this case to justify the changes.

As a side remark: Assuming that we understand your proposal correctly, we believe it would not work as expected in the mentioned DenseVector example. Given the following hypothetical implementation:

using columnVector = std::false_type;
using rowVector    = std::true_type;

template< typename VT, typename TF >  // <- Uses types instead of boolean values
class DenseVector
{};

template< typename T, typename TF >  // <- Uses types instead of boolean values
class DynamicVector : public DenseVector< DynamicVector<T,TF>, TF >
{
   // Details omitted
};

template <template <typename...> class U, typename T>
struct is_a : std::false_type {};

template <template <typename...> class U, typename... Args>
struct is_a<U, U<Args...>> : std::true_type {};

We would expect the following line of code to evaluate to true:

using VectorType = DynamicVector<int,columnVector>;  // The concrete vector type

constexpr bool res = is_a<DenseVector,VectorType>::value;

However, it evaluates to false, since in templates inheritance relationships are not taken into account. Since most type traits in Blaze ask for a category (IsDenseVector, IsSparseMatrix, ...) or a property (IsRowMajorMatrix, IsLower, ...), we believe that only a few type traits could be replaced. From our point of view this is one more argument why we would like to avoid this enormous refactoring.

We would like to close this issue due to the unfavorable ratio between effort and benefit. However, we are interested in whether you miss any specific type trait(s) that we could add. Apart from that we hope that although it may be inconvenient it is still acceptable.

Best regards,

Klaus!

[Nils Deppe]

Hi Klaus,

I'm sure you're not surprised that I strongly disagree with your view, but alas it is not my library :) Feel free to close the feature request.

Best,

Nils

[Klaus Iglberger]

Hi!

I'm not surprised you have a different point of view :-)

But I would have expected that you try to explain how is_a works as a replacement of IsDenseVector (the example that you initially posted and that I tried to implement). I'm also eager to learn how it can replace a type trait that checks for a property (e.g. IsRowVector, IsRowMajorMatrix, IsLower, ...). Could you give an example (one is perfectly enough)?

Best regards,

Klaus!

[Nils Deppe]

Hi Klaus,

The big win is for identifying types like I had in my first post. It wouldn't help with IsRowVector, etc. since these are base classes (if I recall correctly), so there is_base_of should be used.

Nils

[Klaus Iglberger]

Hi!

As stated before we believe this idea is unfeasible. It would require to modify almost all files in Blaze, including all the tests and benchmarks. We estimate that this would take several month of 40h weeks to complete. However, we realize that the proposal is intended to make Blaze a better library. We highly appreciate this! Thanks a lot,

Best regards,

Klaus!