Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more support for ScalarValue::Float16 where Float32 and Float64 are supported #11083

Closed
LorrensP-2158466 opened this issue Jun 23, 2024 · 1 comment · Fixed by #11156
Closed
Labels
enhancement New feature or request

Comments

@LorrensP-2158466
Copy link
Contributor

LorrensP-2158466 commented Jun 23, 2024

Is your feature request related to a problem or challenge?

When fixing #11076, I noticed that not every method in ScalarValue supports a ScalarValue::Float16 while supporting Float32 and Float64:
For example in (from #11076):

pub fn arithmetic_negate(&self) -> Result<Self> {

We see that F32 and F64 are supported, but not F16
ScalarValue::Float64(Some(v)) => Ok(ScalarValue::Float64(Some(-v))),
ScalarValue::Float32(Some(v)) => Ok(ScalarValue::Float32(Some(-v))),

Describe the solution you'd like

  • Every method that supports Float32 and Float64 should support Float16 (including constructors)
  • implement From<f16> for ScalarValue
  • Add tests
  • More to do?

half::f16 supports many of these operations, so I think it is doable. ArrowNativeType and ArrowNativeTypeOp are also implemented so we can use a lot of functions from that trait.

Describe alternatives you've considered

Maybe show in docs why Float16 is not supported in some cases?

Additional context

Can this be a "good first issue" ?
No response

@LorrensP-2158466 LorrensP-2158466 added the enhancement New feature or request label Jun 23, 2024
@Lordworms
Copy link
Contributor

Lordworms commented Jun 23, 2024

I'll take it since it is an issue related to me in #10763

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants