Skip to content

Commit

Permalink
Make the binary multiplication safe for IEEE special values.
Browse files Browse the repository at this point in the history
We can't just skip processing of a structural zero if the other value
might be, e.g., an NaN or Inf, as the product would be a NaN.
  • Loading branch information
LTLA committed May 23, 2024
1 parent 5621240 commit 3ac615d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
10 changes: 8 additions & 2 deletions include/tatami/isometric/binary/arithmetic_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,14 @@ class DelayedBinaryIsometricArithmetic {

template<typename Value_, typename Index_>
Index_ sparse(bool, Index_, const SparseRange<Value_, Index_>& left, const SparseRange<Value_, Index_>& right, Value_* value_buffer, Index_* index_buffer, bool needs_value, bool needs_index) const {
// Don't bother storing an explicit zero for MULTIPLY operations when either entry is zero.
constexpr bool must_have_both = (op_ == ArithmeticOperation::MULTIPLY);
// Technically, MULTIPLY could skip processing if either is a zero.
// However, this is not correct if the other value is an NaN/Inf, as
// the product would be a NaN, not a zero; so we have to err on the
// side of caution of attemping the operation.
constexpr bool must_have_both = (op_ == ArithmeticOperation::MULTIPLY &&
!std::numeric_limits<Value_>::has_quiet_NaN &&
!std::numeric_limits<Value_>::has_infinity);

return delayed_binary_isometric_sparse_operation<must_have_both>(
left,
right,
Expand Down
4 changes: 3 additions & 1 deletion include/tatami/isometric/binary/boolean_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ struct DelayedBinaryIsometricBoolean {

template<typename Value_, typename Index_>
Index_ sparse(bool, Index_, const SparseRange<Value_, Index_>& left, const SparseRange<Value_, Index_>& right, Value_* value_buffer, Index_* index_buffer, bool needs_value, bool needs_index) const {
// Don't bother storing an explicit zero for AND operations when either entry is zero.
// Don't bother storing an explicit zero for AND operations when either
// entry is zero. This should be NaN-safe as NaNs are truthy, so
// applying AND on that would just be false anyway.
constexpr bool must_have_both = (op_ == BooleanOperation::AND);
return delayed_binary_isometric_sparse_operation<must_have_both>(
left,
Expand Down

0 comments on commit 3ac615d

Please sign in to comment.