Add 1.1 binary reader support for floats #756
Conversation
src/lazy/binary/raw/v1_1/reader.rs
Outdated
| macro_rules! assert_closely_eq { | ||
| ($x:expr, $y:expr, $d:expr) => { | ||
| let (a, b) = ($x, $y); | ||
| if ((a - b).abs() >= $d) { | ||
| panic!( | ||
| "float not close enough to expected value: left: {}, right: {}", | ||
| a, b | ||
| ); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
For serialization/deserialization, I don't think we should be considering any epsilon value. A round trip of a float value should result in a value that is equivalent to the original value, and (for non-NaN values) I that means we should expect actual equality of the actual and expected values.
There was a problem hiding this comment.
+1. If roundtripping from f64 -> f32 -> f64 results in a different value, then f32 isn't a valid encoding of that value. You can use IonEq::ion_eq to compare floats with the expected NaN handling.
There was a problem hiding this comment.
Ah yea, thank you for calling this out. I had originally intended to round-trip "friendly" float literals and use this so I wasn't using the true float value for comparison. I didn't go that route, and forgot to remove this.
nirosys
force-pushed
the
rgiliam/1_1_binary_reader_floats
branch
2 times, most recently
from
30771a9 to
0f301c8
Compare
| @@ -109,6 +110,8 @@ impl Header { | |||
| use LengthType::*; | |||
| match (self.ion_type_code, self.length_code) { | |||
| (OpcodeType::Boolean, 0xE..=0xF) => InOpcode(0), | |||
| (OpcodeType::Float, 0xA) => InOpcode(0), | |||
| (OpcodeType::Float, 0xB..=0xD) => InOpcode(1 << (self.length_code - 0xA)), | |||
There was a problem hiding this comment.
1 << (self.length_code - 0xA)
Nice!
nirosys
force-pushed
the
rgiliam/1_1_binary_reader_floats
branch
from
0f301c8 to
9c9166d
Compare
nirosys
force-pushed
the
rgiliam/1_1_binary_reader_floats
branch
from
9c9166d to
91b37d7
Compare
Issue #, if available: #662
Description of changes:
Add support for floats to the 1.1 binary raw reader.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.