Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Added support to read and write f16 #1051

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Added support to read and write f16 #1051

merged 1 commit into from
Jun 13, 2022

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Jun 6, 2022

Some stats: the size of the lib (in .so) is increased by 90Kb

  • main .so: 9.384.144
  • this .so: 9.492.912

it adds no extra dependencies.

The idea is to provide the very minimal functionality to read and write f16 from and to IPC, since f16 is not really used in Rust.

Users that wish more functionality over f16, can use half's f16::from_bits and to_bits:

use arrow2::array::Float16Array;
use arrow2::types::f16;

fn main() {
    let data = vec![f16(half::f16::from_f32(0.1f32).to_bits()); 10];
    let array = Float16Array::from_vec(data);
    println!("{:?}", array);
}

However, when you only need to read f16 from IPC and convert it to f32, you can use arrow2::types::f16::as_f32(), e.g.

use arrow2::array::{Float16Array, Float32Array};
use arrow2::datatypes::DataType;
use arrow2::types::f16;

fn main() {
    let array = Float16Array::from_vec(vec![f16(100); 10]);

    let f32_values = array
        .values()
        .iter()
        .map(|x| x.as_f32())
        .collect::<Vec<_>>();

    let new_array = Float32Array::new(
        DataType::Float32,
        f32_values.into(),
        array.validity().cloned(),
    );
    println!("{:?}", new_array);
}

Closes #1050

@jorgecarleitao jorgecarleitao added the feature A new feature label Jun 6, 2022
@jorgecarleitao jorgecarleitao mentioned this pull request Jun 6, 2022
@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #1051 (d2f5935) into main (e0f4cee) will increase coverage by 0.17%.
The diff coverage is n/a.

❗ Current head d2f5935 differs from pull request most recent head 80cecb5. Consider uploading reports for the commit 80cecb5 to get more accurate results

@@            Coverage Diff             @@
##             main    #1051      +/-   ##
==========================================
+ Coverage   81.13%   81.31%   +0.17%     
==========================================
  Files         365      365              
  Lines       34921    34902      -19     
==========================================
+ Hits        28334    28380      +46     
+ Misses       6587     6522      -65     
Impacted Files Coverage Δ
src/array/primitive/mod.rs 84.39% <0.00%> (-1.33%) ⬇️
src/compute/cast/dictionary_to.rs 26.00% <0.00%> (-0.81%) ⬇️
src/io/ipc/write/schema.rs 98.00% <0.00%> (-0.41%) ⬇️
src/io/ipc/read/schema.rs 93.64% <0.00%> (-0.34%) ⬇️
src/array/ffi.rs 86.95% <0.00%> (ø)
src/ffi/bridge.rs 85.00% <0.00%> (ø)
src/ffi/stream.rs 67.80% <0.00%> (ø)
src/io/ipc/mod.rs 40.00% <0.00%> (ø)
src/scalar/mod.rs 82.97% <0.00%> (ø)
src/scalar/list.rs 96.87% <0.00%> (ø)
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7587a27...80cecb5. Read the comment docs.

@ritchie46
Copy link
Collaborator

Right, so we don't have to implement the full array trait and not all compute and thus can save binary bloat?

@jorgecarleitao
Copy link
Owner Author

jorgecarleitao commented Jun 6, 2022

This does implement the trait for f16, mutable and non-mutable, growable, etc. The primary concern here was to not bring half's dependency to core, which I believe is not justified just to read f16 from IPC.

In other words, our core has so few requirements that we can get away with 150 LOC to support a primitive type in core. ^^

@ritchie46
Copy link
Collaborator

In other words, our core has so few requirements that we can get away with 150 LOC to support a primitive type in core. ^^

That certainly is impressive!

I guess we only need to watch out to not include it in a macro! that downcasts to primitive for any of the compute kernels. We could accidentally compile for f16 as well, whereas that binary code would not be used.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Float16 Support
2 participants