search

milankl / BitInformation.jl

Information between bits and bytes.
MIT License
30 stars 3 forks source link

Incorrect round away from zero for keepbits=significand_bits #36

Closed milankl closed 2 years ago

milankl commented 2 years ago

_Originally posted by @milankl in https://github.com/observingClouds/bitinformation_pipeline/issues/29#issuecomment-1091691892_

In Ryan's numcodecs.bitround https://github.com/zarr-developers/numcodecs/pull/299 it seems that a simple escape is used for keepbits=23 (because no rounding should take place).

def encode(self, buf):
        if self.keepbits == 23:
            return buf

I can easily add that for BitInformation.jl too. Because at the moment we have

julia> bitstring.(A,:split)
10-element Vector{String}:
 "0 01111101 00100011110101011001100"
 "1 01111101 11111100000100010010110"
 "0 01111111 00100110111101011010101"
 "0 01111100 00000101000001101001010"
 "1 01111111 10100000111000010100000"
 "0 01111100 01011110111011000110101"
 "0 01111010 01100000001000101011100"
 "1 01111110 00101101011101001110111"
 "1 01111101 10000010000111001000111"
 "1 01111101 11000011000111110011100"

julia> bitstring.(round(A,22),:split)    # keepbits=22, all correct
10-element Vector{String}:
 "0 01111101 00100011110101011001100"    # no rounding
 "1 01111101 11111100000100010010110"    # no rounding
 "0 01111111 00100110111101011010100"    # round to zero=even (tie)
 "0 01111100 00000101000001101001010"    # no rounding
 "1 01111111 10100000111000010100000"    # no rounding
 "0 01111100 01011110111011000110100"    # round to zero=even (tie)
 "0 01111010 01100000001000101011100"    # no rounding
 "1 01111110 00101101011101001111000"    # round away from zero (with carry)
 "1 01111101 10000010000111001001000"    # round away from zero (with carry)
 "1 01111101 11000011000111110011100"    # no rounding

julia> bitstring.(round(A,23),:split)    # keepbits=23
10-element Vector{String}:
 "0 01111101 00100011110101011001100"    # no rounding, correct
 "1 01111101 11111100000100010010110"    # no rounding, correct
 "0 01111111 00100110111101011010110"    # round away from zero, incorrect
 "0 01111100 00000101000001101001010"    # no rounding, correct
 "1 01111111 10100000111000010100000"    # no rounding, correct
 "0 01111100 01011110111011000110110"    # round away from zero, incorrect
 "0 01111010 01100000001000101011100"    # no rounding, correct
 "1 01111110 00101101011101001111000"    # round away from zero, incorrect
 "1 01111101 10000010000111001001000"    # round away from zero, incorrect
 "1 01111101 11000011000111110011100"    # no rounding, correct

Meaning for the edge case of keepbits=23 there's still some rounding away from zero possible, which obviously shouldn't happen. I'll see in a patch release how to best deal with that

milankl commented 2 years ago

Instead of escaping when keepbits == significand_bits(T) I've added one line to get_shift

function get_shift(::Type{T},keepbits::Integer) where {T<:Base.IEEEFloat}
    shift = Base.significand_bits(T) - keepbits     # normal case
    shift -= keepbits == Base.significand_bits(T)   # to avoid round away from 0
    return shift
end

sure whether the == check is here or before calling round! probably doesn't matter but it avoids high-level branching