Since this is part of the delta protocol, there is a case for this being a place where exact floating-point comparison is reasonable ~> need to tell the static analysis tools to ignore it here. Alternatively, since in the binary format, we're transmitting these as fixed-point ~> could add appropriate comparison functions to dataio and call into those (though the JSON format's comparison would still need to be exact).
We could also not diff float fields at all and save those bits in the header (though we don't currently have a mechanism to exempt only individual fields from delta; would have to add that).
Reply To alienvalkyrie
We could also not diff float fields at all and save those bits in the header (though we don't currently have a mechanism to exempt only individual fields from delta; would have to add that).
I guess there would be also an option to just set the bit always, with no check involved at all.
Reply To alienvalkyrie
Since this is part of the delta protocol, there is a case for this being a place where exact floating-point comparison is reasonable ~> need to tell the static analysis tools to ignore it here.
Actually, there's a bug there. The delta protocol is interested about whether the value to be carried over the network differs from the one sent earlier. So we should not compare exact floating point values, but the rounded values that are actually sent.
Reply To cazfi
So we should not compare exact floating point values, but the rounded values that are actually sent.
Probably most reasonable to go with this. Although this does change the behavior of the JSON protocol, which still transmits exact floating point values; in cases where the exact value is different, but the rounded value is not (so the binary protocol would've unnecessarily sent the same rounded value again).
The alternative to avoid this would've been to add aforementioned context-aware comparison methods to the dataio layer, which know whether they're dealing with a JSON connection and can do an exact comparison in that case; but that feels unnecessarily clunky, especially since there's still debate to be had about whether we want to keep the fixed-point handling we have.
CodeQL about main branch: