clockworklabs / clockworklabs/SpacetimeDB

Decide on a consistent semantics for float eq/ord

Open
#1,579 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
25.2k
Forks
1.1k
Avg merge
2d 7h
Merged PRs (30d)
46

Description

Currently, AlgebraicValue::F32/F64 uses decorum::Total<f32> and Total<f64> as a way to get Ord and PartialEq implementations for floats. The Ord impl eventually uses decorum::FloatOrd and decorum::FloatEq. For FloatEq, this eventually results in calling to_canonical_bits, which is implemented here. As explained in the FloatOrd and FloatEq docs, one of their properties is that they treat all NaNs as equal. These semantics for floats are used for AlgebraicValue, eq_bsatn, and in eq_to_pv.

Meanwhile, the spacetimedb_table::eq implementation compares bytes directly, which is ostensibly compatible with the {f32, f64}::total_cmp methods, which behave according to the totalOrd predicate defined by IEEE 754 FP standard, and unlike the behavior of FloatOrd.

We thus have an inconsistency in our implementation and should pick one.
For performance, it seems like total_cmp would be the winner, as it allows us to keep comparing bytes directly for VLOs.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing AlgebraicValue::F32/F64 and the eq_bsatn and eq_to_pv paths with spacetimedb_table::eq. Read the referenced decorum FloatOrd, FloatEq, and total_cmp semantics, including the byte comparisons used for VLOs. Done means choosing one float equality and ordering policy and making the affected implementations consistent with it.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.