Implement ArrowNumericType for Float16Type #2810
Conversation
| @@ -333,8 +333,7 @@ where | |||
| // process data in chunks of 64 elements since we also get 64 bits of validity information at a time | |||
|
|
|||
| // safety: result is newly created above, always written as a T below | |||
| let mut result_chunks = | |||
| unsafe { result.typed_data_mut().chunks_exact_mut(64) }; | |||
| let mut result_chunks = result.typed_data_mut().chunks_exact_mut(64); | |||
There was a problem hiding this comment.
These are drive-by fixes that date from #1866
There was a problem hiding this comment.
👍 -- as below we probably can remove the old //safety comments too
| impl ArrowNumericType for Float16Type {} | ||
|
|
||
| #[cfg(feature = "simd")] | ||
| impl ArrowNumericType for Float16Type { |
There was a problem hiding this comment.
Perhaps an expert in SIMD / GPU could give this a look as well (no need to hold merging up) -- I though f16 was primarily useful when doing SIMD/GPU so perhaps there some special support here we could use
In any event, this seems like a good step forward to me 👍
Perhaps @HaoYang670 or @Jimexist (who originally contributed f16 support in #890) might have some ideas
| a.values().iter().zip(b.values()).map(|(a, b)| a + b), | ||
| ); | ||
|
|
||
| let c = add(&a, &b).unwrap(); |
| @@ -1938,7 +1938,7 @@ where | |||
| let simd_right = T::init(right); | |||
|
|
|||
| // safety: result is newly created above, always written as a T below | |||
There was a problem hiding this comment.
we probably don't need the //safety comments anymore as we are using a safe API now
| @@ -333,8 +333,7 @@ where | |||
| // process data in chunks of 64 elements since we also get 64 bits of validity information at a time | |||
|
|
|||
| // safety: result is newly created above, always written as a T below | |||
| let mut result_chunks = | |||
| unsafe { result.typed_data_mut().chunks_exact_mut(64) }; | |||
| let mut result_chunks = result.typed_data_mut().chunks_exact_mut(64); | |||
There was a problem hiding this comment.
👍 -- as below we probably can remove the old //safety comments too
|
Benchmark runs are scheduled for baseline = 931c6fc and contender = 9c1748f. 9c1748f is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #.
Rationale for this change
This is mainly so that I can get #2809 in, but adding support for Float16 is important from a coverage perspective.
What changes are included in this PR?
Implements SIMD operations on f16 using the implementation for f32, I suspect although haven't benchmarked, this is faster than a naive implementation that uses a mask of
Simd=f16andSimdMask = boolAre there any user-facing changes?
No