Skip to content

GH-51311: [C++][Compute] Add float16 to FloatingPointTypes - #51330

Open
Reranko05 wants to merge 2 commits into
apache:mainfrom
Reranko05:gh-51311-float16
Open

Reranko05 wants to merge 2 commits into
apache:mainfrom
Reranko05:gh-51311-float16

Conversation

@Reranko05

@Reranko05 Reranko05 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Rationale

FloatingPointTypes() currently contains only float32() and float64(), so float16() is also omitted from NumericTypes() and PrimitiveTypes().

This was noted as a follow-up during the review of #50513.

Changes

  • Add float16() to FloatingPointTypes().
  • NumericTypes() and PrimitiveTypes() automatically include float16() through their existing composition from FloatingPointTypes().

Testing

  • arrow-type-test passes.

GitHub Issue: #51311

@Reranko05
Reranko05 requested a review from pitrou as a code owner September 14, 2026 19:08
Copilot AI lite review requested due to automatic review settings September 14, 2026 19:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #51311 has been automatically assigned in GitHub to PR creator.

@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

This pull request has been automatically closed because you currently have 5 open pull requests, which is more than the limit of 3.

Due to the increase in pull requests opened by AI bots, and in order to keep the review queue manageable, Apache Arrow limits contributors without write access to at most 3 concurrently open pull requests. This helps make sure each pull request gets the attention it needs and that work in progress does not go stale.

Once one of your other open pull requests has been merged or closed, you are welcome to reopen this one.

See also:

@Reranko05

Copy link
Copy Markdown
Collaborator Author

Once #50513 is merged, we can verify that the updated type list works correctly.

@pitrou

pitrou commented Sep 17, 2026

Copy link
Copy Markdown
Member

@Reranko05 , as I expected, this is breaking a lot of existing tests for functionality that doesn't accept Float16 currently.

The easy way out is to avoid running those tests on Float16, but it would be nicer of course to progressively have all the tested functionality accept Float16.

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@pitrou Okay, so should I update the affected tests to avoid float16 for now?

@pitrou

pitrou commented Sep 17, 2026

Copy link
Copy Markdown
Member

Yes, probably. Perhaps using a test-only function that would return floating-point types without Float16.

Copilot AI review requested due to automatic review settings September 17, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@pitrou NumericTypes() gets float16 indirectly through g_floating_types. Once float16 is included, production registration paths iterating over NumericTypes() can pass Type::HALF_FLOAT to ArithmeticExecFromOp(), which currently only has cases for FLOAT and DOUBLE (float32/float64). So this needs to be handled separately from the test-only exclusions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants