westonpace commented on code in PR #33775:
URL: https://github.com/apache/arrow/pull/33775#discussion_r1084073883
##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -790,6 +796,30 @@ ExtensionIdRegistry::SubstraitCallToArrow
DecodeOptionlessUncheckedArithmetic(
};
}
+ExtensionIdRegistry::SubstraitCallToArrow DecodeBinaryRoundingMode(
+ const std::string& function_name) {
+ return [function_name](const SubstraitCall& call) ->
Result<compute::Expression> {
+ ARROW_ASSIGN_OR_RAISE(
+ compute::RoundMode round_mode,
+ ParseOptionOrElse(
+ call, "rounding", kRoundModeParser,
+ {compute::RoundMode::DOWN, compute::RoundMode::UP,
+ compute::RoundMode::TOWARDS_ZERO,
compute::RoundMode::TOWARDS_INFINITY,
+ compute::RoundMode::HALF_DOWN, compute::RoundMode::HALF_UP,
+ compute::RoundMode::HALF_TOWARDS_ZERO,
+ compute::RoundMode::HALF_TOWARDS_INFINITY,
compute::RoundMode::HALF_TO_EVEN,
+ compute::RoundMode::HALF_TO_ODD},
+ compute::RoundMode::HALF_TOWARDS_INFINITY));
Review Comment:
Yes, for some reason I thought the spec declared that the first option
specified should be the default. However, upon re-reading, it appears I was
incorrect.
> Options can be left unspecified and the consumer is free to choose which
implementation to use.
That being said, HALF_TO_EVEN is the default in Arrow if you are not using
Substrait and it is the default IEEE rounding mode. I don't know exactly why
but, after some googling, it seems that the most likely reason is that it tends
to be the most unbiased:
https://stackoverflow.com/questions/45129046/why-floats-rounding-use-round-to-even
I would agree though, that AWAY_FROM_ZERO would be my intuitive expectation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
