jatinchowdhury18 / RTNeural

Real-time neural network inferencing
BSD 3-Clause "New" or "Revised" License
582 stars 58 forks source link

Update xsimd for Android support. #81

Closed atsushieno closed 1 year ago

atsushieno commented 1 year ago

context: https://github.com/jatinchowdhury18/RTNeural/issues/80

This makes RTNeural just build on Android without disabling optimized exp and exp10.

codecov-commenter commented 1 year ago

Codecov Report

Merging #81 (7f28525) into main (063810e) will decrease coverage by 0.06%. The diff coverage is 93.02%.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
- Coverage   96.63%   96.57%   -0.07%     
==========================================
  Files          29       30       +1     
  Lines        2437     2480      +43     
==========================================
+ Hits         2355     2395      +40     
- Misses         82       85       +3     
Impacted Files Coverage Δ
RTNeural/common.h 93.89% <ø> (ø)
RTNeural/xsimd-legacy/algorithms/algorithms.hpp 93.02% <93.02%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jatinchowdhury18 commented 1 year ago

Thanks for the PR!

This is a bit of a tricky one since the XSIMD folks removed xsimd::transform and xsimd::reduce without really offering a replacement. I'd rather not use the std:: replacements since that would force RTNeural to depend on C++17, and I'd definitely prefer to keep C++14 compatibility. Additionally, the std:: methods would likely be less performant in this context since they lose the explicit vectorization that the XSIMD methods provided.

Maybe the best option is to just copy this file to somewhere inside the RTNeural source tree (maybe RTNeural/RTNeural/xsimd-algorithms/algorithms.hpp), leave a comment saying why it's there, and then include it in common.h under the XSIMD section. Do you think that would work?

atsushieno commented 1 year ago

Yes, that sounds perfect. I will update this to make the changes later, but if you make it I appreciate that!

atsushieno commented 1 year ago

The changes are made as ^

jatinchowdhury18 commented 1 year ago

Awesome, thanks! I decided to bump the XSIMD version up to 10.0.0, and use the `xsimd_api.hpp" header from within XSIMD, just to avoid having duplicates of any headers.

Once the CI is happy with everything, then I'll go ahead and merge this!