[Type] Array/Vec: small refresh for modern C++ #5907
Conversation
fredroy
added
pr: status to review
|
[ci-build][with-all-tests] |
| for (Size i=1; i<N; i++) | ||
| r += this->elems[i]*this->elems[i]; | ||
| return r; | ||
| return std::inner_product(elems.begin(), elems.end(), elems.begin(), ValueType{}); |
There was a problem hiding this comment.
We can ask the question of the performances...
There was a problem hiding this comment.
We can ask the question of the performances...
Usually people (and IA...) advise to use stl functions because it may help the compiler more (vectorization and stuff). But of course it depends of the implementation of the libstl itself... I will try to do some bench tho
There was a problem hiding this comment.
https://quick-bench.com/q/WGEPcgHll4O2cAl8Ms-VoyRBn7I
At -O3, all config (clang/gcc & libstdc++/libc++) give same result. But will less optimization the loop is faster.
Should I roll back the implementation ?
| TEST(VecTest, setFromOtherVec) | ||
| { | ||
| sofa::type::Vec<3, double> v1(1.0, 2.0, 3.0); | ||
| sofa::type::Vec<5, double> v; | ||
| v.set(v1); | ||
| EXPECT_EQ(v[0], 1.0); | ||
| EXPECT_EQ(v[1], 2.0); | ||
| EXPECT_EQ(v[2], 3.0); | ||
| } |
There was a problem hiding this comment.
set here is purposefully tested with a smaller vector?
There was a problem hiding this comment.
Yes but I can add other set() cases as well
Co-authored-by: Themis Skamagkis <70031729+th-skam@users.noreply.github.com>
| for (Size i=0; i<N; i++) | ||
| sum += this->elems[i]; | ||
| return sum; | ||
| return std::accumulate(elems.begin(), elems.end(), ValueType{}); |
There was a problem hiding this comment.
3 participants
using std algos, clamp, etc and some keywords noexcept.
and fixed_array_algo, removing some useless templates
and some (generated but modified) unit tests for vec ,as there were almost none ? 🤔
Note that fixed_array_algo is only used in RGBAColor, at least in the standard code base.
[with-all-tests]
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if