Open AnderOne opened 5 years ago
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cdydM8D_tsZUuEoR6_dPvAhsrXdz9ks5vbhYrgaJpZM4cSZjn .
I could suggest make TinyVector as scalar type without expression template support. Then all arithmetic operators between TinyVectors will return TinyVector as value. Also assignment operator can get appropriate specialization. It's simplest way to solve this problem.
TinyVector supports templated, vectorized expressions. Its performance is crucial for heavy use of fixed-size vectors (e.g. 3-d vectors etc). std::array does not support any of that.
While this behavior for multicomponent arrays clearly is a bug, if you don't want the expression template functionality for TinyVector it's trivial to write your own vector class and use that instead.
On Fri, Mar 29, 2019 at 3:43 AM Elizabeth Fischer notifications@github.com wrote:
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread < https://github.com/notifications/unsubscribe-auth/AB1cdydM8D_tsZUuEoR6_dPvAhsrXdz9ks5vbhYrgaJpZM4cSZjn
.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478002134, or mute the thread https://github.com/notifications/unsubscribe-auth/AFK8GDGLIm65TtgqlhKlr2tQi5qroH05ks5vbhh2gaJpZM4cSZjn .
Can you elaborate further? My experience with TinyVector is it's used for small (1-11 element) vectors of array bounds, etc; and in that case, it seems that std::array would do just as well. Is TinyVector used for other purposes as well?
-- Elizabeth
On Fri, Mar 29, 2019 at 10:08 PM Patrik Jonsson notifications@github.com wrote:
TinyVector supports templated, vectorized expressions. Its performance is crucial for heavy use of fixed-size vectors (e.g. 3-d vectors etc). std::array does not support any of that.
While this behavior for multicomponent arrays clearly is a bug, if you don't want the expression template functionality for TinyVector it's trivial to write your own vector class and use that instead.
On Fri, Mar 29, 2019 at 3:43 AM Elizabeth Fischer < notifications@github.com> wrote:
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread <
.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478002134, or mute the thread < https://github.com/notifications/unsubscribe-auth/AFK8GDGLIm65TtgqlhKlr2tQi5qroH05ks5vbhh2gaJpZM4cSZjn
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478196685, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cd0IWYM0sYUrjZmB2SzmZ600Pg1UYks5vbscfgaJpZM4cSZjn .
It seems that I have solved the problem.
But for this purpose I had to use the means of C++11 (from
I'm not sure that my code works correctly in all possible cases, but it passes all existing tests.
If you decide that it's useful, I could send a pull request.
A PR would be great!
On Fri, Apr 5, 2019 at 8:47 AM AnderOne notifications@github.com wrote:
It seems that I have solved the problem. But for this purpose I had to use the means of C++11 (from
): https://github.com/AnderOne/blitz/commits/test_for_expr I'm not sure that my code works correctly in all possible cases, but it passes all existing tests.
If you decide that it's useful, I could send a pull request.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-480262674, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cdySdNPjhAJDAAt2YwTRRSxMaQxhmks5vd0XbgaJpZM4cSZjn .
PR is here: #109
Sorry for the slow response, I was traveling.
TinyVector is used for the internal offset and bounds calculations in blitz, and for the blocked Array expressions used to facilitate SIMD optimization. But it's also a user-facing class that can be used for any fixed-size vector calculations where high performance is needed. In my physics codes I use TinyVector/TinyMatrix extensively for 3d vector calculations. std::array does not support math operators. Even if it did, it would not have the performance of TinyVector. (Fixed-size expression-template vector/matrix classes that don't allocate any memory dynamically are much more efficient than Array for short lengths, so it is not a substitute either.)
Cheers,
/Patrik
On Mon, Apr 1, 2019 at 9:55 AM Elizabeth Fischer notifications@github.com wrote:
Can you elaborate further? My experience with TinyVector is it's used for small (1-11 element) vectors of array bounds, etc; and in that case, it seems that std::array would do just as well. Is TinyVector used for other purposes as well?
-- Elizabeth
On Fri, Mar 29, 2019 at 10:08 PM Patrik Jonsson notifications@github.com wrote:
TinyVector supports templated, vectorized expressions. Its performance is crucial for heavy use of fixed-size vectors (e.g. 3-d vectors etc). std::array does not support any of that.
While this behavior for multicomponent arrays clearly is a bug, if you don't want the expression template functionality for TinyVector it's trivial to write your own vector class and use that instead.
On Fri, Mar 29, 2019 at 3:43 AM Elizabeth Fischer < notifications@github.com> wrote:
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread <
.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478002134, or mute the thread <
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478196685, or mute the thread < https://github.com/notifications/unsubscribe-auth/AB1cd0IWYM0sYUrjZmB2SzmZ600Pg1UYks5vbscfgaJpZM4cSZjn
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478722774, or mute the thread https://github.com/notifications/unsubscribe-auth/AFK8GEBv6TuwSUzr-SSSOrhquU60N0hmks5vcmRBgaJpZM4cSZjn .
I believe std::array is a fixed size template that doesn’t allocate memory. ???
On Sun, Apr 7, 2019 at 14:36 Patrik Jonsson notifications@github.com wrote:
Sorry for the slow response, I was traveling.
TinyVector is used for the internal offset and bounds calculations in blitz, and for the blocked Array expressions used to facilitate SIMD optimization. But it's also a user-facing class that can be used for any fixed-size vector calculations where high performance is needed. In my physics codes I use TinyVector/TinyMatrix extensively for 3d vector calculations. std::array does not support math operators. Even if it did, it would not have the performance of TinyVector. (Fixed-size expression-template vector/matrix classes that don't allocate any memory dynamically are much more efficient than Array for short lengths, so it is not a substitute either.)
Cheers,
/Patrik
On Mon, Apr 1, 2019 at 9:55 AM Elizabeth Fischer <notifications@github.com
wrote:
Can you elaborate further? My experience with TinyVector is it's used for small (1-11 element) vectors of array bounds, etc; and in that case, it seems that std::array would do just as well. Is TinyVector used for other purposes as well?
-- Elizabeth
On Fri, Mar 29, 2019 at 10:08 PM Patrik Jonsson < notifications@github.com> wrote:
TinyVector supports templated, vectorized expressions. Its performance is crucial for heavy use of fixed-size vectors (e.g. 3-d vectors etc). std::array does not support any of that.
While this behavior for multicomponent arrays clearly is a bug, if you don't want the expression template functionality for TinyVector it's trivial to write your own vector class and use that instead.
On Fri, Mar 29, 2019 at 3:43 AM Elizabeth Fischer < notifications@github.com> wrote:
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread <
.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/blitzpp/blitz/issues/108#issuecomment-478002134 , or mute the thread <
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478196685, or mute the thread <
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478722774, or mute the thread < https://github.com/notifications/unsubscribe-auth/AFK8GEBv6TuwSUzr-SSSOrhquU60N0hmks5vcmRBgaJpZM4cSZjn
.
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-480617693, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cd9iBSuetYAGJtlnekRHTuVWekdsVks5vejqqgaJpZM4cSZjn .
This is true, but it has no math operators, let alone ones using something similar to the blitz ET machinery.
On Sun, Apr 7, 2019, 09:59 Elizabeth Fischer notifications@github.com wrote:
I believe std::array is a fixed size template that doesn’t allocate memory. ???
On Sun, Apr 7, 2019 at 14:36 Patrik Jonsson notifications@github.com wrote:
Sorry for the slow response, I was traveling.
TinyVector is used for the internal offset and bounds calculations in blitz, and for the blocked Array expressions used to facilitate SIMD optimization. But it's also a user-facing class that can be used for any fixed-size vector calculations where high performance is needed. In my physics codes I use TinyVector/TinyMatrix extensively for 3d vector calculations. std::array does not support math operators. Even if it did, it would not have the performance of TinyVector. (Fixed-size expression-template vector/matrix classes that don't allocate any memory dynamically are much more efficient than Array for short lengths, so it is not a substitute either.)
Cheers,
/Patrik
On Mon, Apr 1, 2019 at 9:55 AM Elizabeth Fischer < notifications@github.com
wrote:
Can you elaborate further? My experience with TinyVector is it's used for small (1-11 element) vectors of array bounds, etc; and in that case, it seems that std::array would do just as well. Is TinyVector used for other purposes as well?
-- Elizabeth
On Fri, Mar 29, 2019 at 10:08 PM Patrik Jonsson < notifications@github.com> wrote:
TinyVector supports templated, vectorized expressions. Its performance is crucial for heavy use of fixed-size vectors (e.g. 3-d vectors etc). std::array does not support any of that.
While this behavior for multicomponent arrays clearly is a bug, if you don't want the expression template functionality for TinyVector it's trivial to write your own vector class and use that instead.
On Fri, Mar 29, 2019 at 3:43 AM Elizabeth Fischer < notifications@github.com> wrote:
I had no idea that tinyvector and array ever even mixed like this. Can we get rid of tinyvector in favor of std::array? I think that would be an improvement
On Fri, Mar 29, 2019 at 07:33 AnderOne notifications@github.com wrote:
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 1> DAT(5);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to incorrect behavior!
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
If N-dimensional array is used then it leads to compilation error:
include <blitz/array.h>
include
int main() {
blitz::Array<blitz::TinyVector<double, 2>, 2> DAT(5, 4);
blitz::TinyVector<double, 2> TMP(3, 7);
if false
//Here we construct TinyVector from expression and fill Array. It's right!
DAT = blitz::TinyVector<double, 2> (TMP * 2 - 1);
else
//This code leads to compilation error!
/./blitz/blitz/array/expr.h:194:81: error: no match for call to ‘(const T_expr {aka const blitz::FastTV2Iterator<double, 2>}) (const blitz::TinyVector<int, 2>&)’ T_result operator()(const TinyVector<int, Nrank>& i) const { return iter(i); } /
DAT = TMP * 2 - 1;
endif
std::cout << DAT << std::endl;
return 0;
}
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108, or mute the thread <
.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/blitzpp/blitz/issues/108#issuecomment-478002134 , or mute the thread <
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/blitzpp/blitz/issues/108#issuecomment-478196685 , or mute the thread <
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-478722774, or mute the thread <
.
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-480617693, or mute the thread < https://github.com/notifications/unsubscribe-auth/AB1cd9iBSuetYAGJtlnekRHTuVWekdsVks5vejqqgaJpZM4cSZjn
.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/108#issuecomment-480624023, or mute the thread https://github.com/notifications/unsubscribe-auth/AFK8GDZrSbkvDq-JMvsKUEC7aFP7WRC1ks5vek4mgaJpZM4cSZjn .
My patch doesn't break ET logic. It just make checking for a type conversion between right side expression and Array::T_numtype in a compile time. However, this feature requires C++11.
I known that migrate to a modern C++ repeatedly discussed. But in this case, it really fixes a bug.
On Sun, Apr 7, 2019 at 7:15 PM Patrik Jonsson notifications@github.com wrote:
This is true, but it has no math operators, let alone ones using something similar to the blitz ET machinery.
In my experience, the non-standard TinyVector class requires I frequently copy stuff from std::array in to a TinyVector for dimension sizes, indices, etc. That's the main impetus for moving to std::array. Math operators can be defined on std::array if one wishes. Moreover, I'm wondering if features already provided by Eigen might be a better fit for the more-extensive application of TinyVector described here.
There is a problem with to unwrap of expression templates when we use multicomponent Array on the left side and a vector expression on the right.
If N-dimensional array is used then it leads to compilation error: