A couple of improvements for VexCL utilities

[ddemidov]

• Avoid creating temporaries for vex::max expressions
• Specify queue list when resizing vex::vectors

[roystgnr]

Good fix and good idea; thanks!

I'm not sure how I missed that missing queue list. Maybe it should be possible to disable VexCL's default-queue-list methods & arguments by setting some -DVEXCL_NO_DEFAULT_QUEUE type define? We do something similar in libMesh: novice users and old codes can default to MPI COMM_WORLD, but more robust codes can be tested against a --disable-default-comm-world configured library to give a compile-time error if they accidentally try to use that default. libMesh/libmesh#85

Where we really need to be returning auto is in many of the Antioch routines themselves; I've been holding off on that sweeping edit until we get some newer GPUs with which to benchmark any change in speed. That's going to be complicated by the need to maintain C++2003 compatibility for non-VexCL users, though. I'm currently leaning toward "AUTO_OR(other_return_type)" and "RETURN_DECLTYPE(expression)" macros with different definitions depending on whether ANTIOCH_HAVE_CXX11 is set.

Hmm... and in fact, I'd need to do that right now, too, before I can pull this. I wonder if I can just get rid of our std::max(vex) override and rely on Koenig lookup finding vex::max() when appropriate? I think we've replaced all the "std::max(a,b)" with "using std::max(); max(a,b)" by now.

And... no luck. g++ 4.7.3 tries to use std::max() anyway, which then fails. I'll see if I can figure out the lookup issues and/or add those auto/decltype macros later today.

[ddemidov]

Good fix and good idea; thanks!

I'm not sure how I missed that missing queue list. Maybe it should be
possible to disable VexCL's default-queue-list methods & arguments by
setting some -DVEXCL_NO_DEFAULT_QUEUE type define? We do something similar
in libMesh: novice users and old codes can default to MPI COMM_WORLD, but
more robust codes can be tested against a --disable-default-comm-world
configured library to give a compile-time error if they accidentally try to
use that default. libMesh/libmesh#85

That would be easy to do, and it seems useful for library developers, so
I'll do it, thanks!

Where we really need to be returning auto is in many of the Antioch
routines themselves; I've been holding off on that sweeping edit until we
get some newer GPUs with which to benchmark any change in speed. That's
going to be complicated by the need to maintain C++2003 compatibility for
non-VexCL users, though. I'm currently leaning toward
"AUTO_OR(other_return_type)" and "RETURN_DECLTYPE(expression)" macros with
different definitions depending on whether ANTIOCH_HAVE_CXX11 is set.

Hmm... and in fact, I'd need to do that right now, too, before I can pull
this.

Vexcl needs c++11 anyway, so probably this particular change is safe
enough.

I wonder if I can just get rid of our std::max(vex) override and rely on
Koenig lookup finding vex::max() when appropriate? I think we've replaced
all the "std::max(a,b)" with "using std::max(); max(a,b)" by now.

I'll try to look at this issue next week. Can you specify a particular
target that breaks without your override?

And... no luck. g++ 4.7.3 tries to use std::max() anyway, which then
fails. I'll see if I can figure out the lookup issues and/or add those
auto/decltype macros later today.

[ddemidov]

Regarding ADL failure: I give up. vec_compare<Eigen::Array<vex::vector<>>> compiles when using std::max; is commented out, or when vex::max is explicitly used. But when unqualified max is used, and std::max is in the scope, it always gets selected (both with g++ and clang++).

[roystgnr]

Regarding ADL failure: is it possible that vex::max(T a, T b) is only defined for vex::vector_expression or some such data type that requires implicit conversion from vex::vector? If so then I think we'd actually expect std::max(vex::vector, vex::vector) to be preferred. Koenig lookup adds vex::max to the set of overloads to be considered, but overload resolution then still prefers an exact match over a match which requires a conversion.

[ddemidov]

vex::max()'s arguments are templated (see here), so it should not be the case. Also I get max is ambiguous error when I try to compile the following:

#include <algorithm>

namespace NS {
    struct S {};

    template <class T>
    T max(const T& a, const T& b) {
        return a;
    }
}

int main() {
    using std::max;
    NS::S a, b, c = max(a, b);
}

So it seems there is another layer of problems with this issue.

[ddemidov]

Ah, but std::max gets preferred in this case (and I get error no operator<() for NS::S both with g++ and clang):

#include <algorithm>

namespace NS {
    struct S {};

    template <class T1, class T2> // <-- Here is the change
    T1 max(const T1& a, const T2& b) {
        return a;
    }
}

int main() {
    using std::max;
    NS::S a, b, c = max(a, b);
}

[roystgnr]

Is that the situation with vex::max? Independent template typenames for the two arguments?

[ddemidov]

It is. I can get different types for max arguments, because arguments are expression templates.

[roystgnr]

Naturally. Now I'm not sure why I've never run into this in my own code. I typically have specialized max(MyType,MyType); does that get preferred over the real std:: in the max(MyType,MyType) case?

[ddemidov]

Hmm, with the following I get max is ambiguous:

#include <algorithm>

namespace NS {
    template <class T> struct S {};

    template <class T1, class T2>
    S<T1> max(const S<T1>& a, const S<T2>& b) {
        return a;
    }
}

int main() {
    using std::max;
    NS::S<int> a, b, c = max(a, b);
}

[ddemidov]

Quick grep through antioch code showed that its always max(MyClass<T>, MyClass<T>).

[ddemidov]

And there is no ambiguity here (it compiles fine):

#include <algorithm>

namespace NS {
    template <class T> struct S {};

    template <class T>
    S<T> max(const S<T>& a, const S<T>& b) {
        return a;
    }
}

int main() {
    using std::max;
    NS::S<int> a, b, c = max(a, b);
}

[ddemidov]

So max(C<T>, C<T>) is specialization of std::max, and max(C<T1>, C<T2>) is an ambiguous overload.

[ddemidov]

You'll probably need an Antioch::max<T1, T2> to resolve this.

[pbauman]

@roystgnr, did this get hit along with others in #12?

[roystgnr]

Hmm... not quite. The infrastructure is there now for doing max properly but we're not hitting it yet. Plus I keep forgetting to cherry-pick the queue list bugfix. I'll get those now.

[roystgnr]

Actually, we are now doing max as well as this patch does - assuming the inputs are full vex::vector objects, but returning an auto expression template where possible.

That's only part way towards doing max (and other functions) properly, though - we really want to be able to accept any expression template as inputs too. I haven't yet settled on how to write template functions which match all possible template expressions from one library but which don't end up creating ambiguous matches with objects and expressions from another library. Our switch to "Antioch::ant_foo()" shims might make that much easier now though.

But that ought to be a new issue; I think we're up to speed on this one.