Skip to content

Commit 5a1eaaf

Browse files
authored
Merge pull request #133 from flyingrobots/F32Scalar/NaNs
feat(rmg-core): Canonicalize F32Scalar NaNs, update MSRV to 1.83.0, a…
2 parents e856187 + 8addc0b commit 5a1eaaf

File tree

9 files changed

+178
-20
lines changed

9 files changed

+178
-20
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/rmg-core/Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
name = "rmg-core"
55
version = "0.1.0"
66
edition = "2021"
7-
rust-version = "1.71.1"
7+
rust-version = "1.83.0"
88
description = "Echo core: deterministic typed graph rewriting engine"
99
license = "Apache-2.0"
1010
repository = "https://github.com/flyingrobots/echo"
@@ -20,7 +20,6 @@ thiserror = "1.0"
2020
hex = { version = "0.4", optional = true }
2121
serde = { version = "1.0", features = ["derive"], optional = true }
2222
serde_json = { version = "1.0", optional = true }
23-
once_cell = "1.19"
2423
rustc-hash = "2.1.1"
2524

2625
[dev-dependencies]

crates/rmg-core/src/constants.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots>
33
//! Canonical digests and constants used across the engine.
4-
use once_cell::sync::Lazy;
4+
use std::sync::LazyLock;
55

66
use crate::ident::Hash;
77

88
/// BLAKE3 digest of an empty byte slice.
99
///
1010
/// Used where canonical empty input semantics are required.
11-
pub static BLAKE3_EMPTY: Lazy<Hash> = Lazy::new(|| blake3::hash(&[]).into());
11+
pub static BLAKE3_EMPTY: LazyLock<Hash> = LazyLock::new(|| blake3::hash(&[]).into());
1212

1313
/// Canonical digest representing an empty length-prefix list: BLAKE3 of
1414
/// `0u64.to_le_bytes()`.
1515
///
1616
/// Used for plan/decision/rewrites digests when the corresponding list is empty.
17-
pub static DIGEST_LEN0_U64: Lazy<Hash> = Lazy::new(|| {
17+
pub static DIGEST_LEN0_U64: LazyLock<Hash> = LazyLock::new(|| {
1818
let mut h = blake3::Hasher::new();
1919
h.update(&0u64.to_le_bytes());
2020
h.finalize().into()

crates/rmg-core/src/math/scalar.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,15 @@ impl F32Scalar {
131131
///
132132
/// Canonicalizes `-0.0` to `+0.0` to ensure deterministic zero handling.
133133
pub const fn new(num: f32) -> Self {
134-
Self { value: num + 0.0 }
134+
if num.is_nan() {
135+
// Canonical NaN: 0x7fc00000 (Positive Quiet NaN)
136+
Self {
137+
value: f32::from_bits(0x7fc0_0000),
138+
}
139+
} else {
140+
// Canonicalize -0.0 to +0.0
141+
Self { value: num + 0.0 }
142+
}
135143
}
136144
}
137145

crates/rmg-core/tests/determinism_policy_tests.rs

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@
33

44
#![allow(missing_docs)]
55

6-
// use rmg_core::math::scalar::F32Scalar;
7-
// use rmg_core::math::Scalar;
6+
use rmg_core::math::scalar::F32Scalar;
7+
use rmg_core::math::Scalar;
88

99
// NOTE: These tests describe the intended strict determinism policy.
1010
// They currently fail because F32Scalar only canonicalizes -0.0.
1111
// They are commented out until the "CanonicalF32" work (Issue #XXX) is landed.
1212

13-
/*
1413
#[test]
1514
fn test_policy_nan_canonicalization() {
1615
// Construct different NaNs
@@ -27,12 +26,29 @@ fn test_policy_nan_canonicalization() {
2726
// All must be bitwise identical to the canonical NaN
2827
let canonical_bits = 0x7fc00000; // Standard positive quiet NaN
2928

30-
assert_eq!(s1.to_f32().to_bits(), canonical_bits, "Positive qNaN not canonicalized");
31-
assert_eq!(s2.to_f32().to_bits(), canonical_bits, "Negative qNaN not canonicalized");
32-
assert_eq!(s3.to_f32().to_bits(), canonical_bits, "Signaling NaN not canonicalized");
33-
assert_eq!(s4.to_f32().to_bits(), canonical_bits, "Payload NaN not canonicalized");
29+
assert_eq!(
30+
s1.to_f32().to_bits(),
31+
canonical_bits,
32+
"Positive qNaN not canonicalized"
33+
);
34+
assert_eq!(
35+
s2.to_f32().to_bits(),
36+
canonical_bits,
37+
"Negative qNaN not canonicalized"
38+
);
39+
assert_eq!(
40+
s3.to_f32().to_bits(),
41+
canonical_bits,
42+
"Signaling NaN not canonicalized"
43+
);
44+
assert_eq!(
45+
s4.to_f32().to_bits(),
46+
canonical_bits,
47+
"Payload NaN not canonicalized"
48+
);
3449
}
3550

51+
/*
3652
#[test]
3753
fn test_policy_subnormal_flushing() {
3854
// Smallest positive subnormal: 1 bit set in mantissa, 0 exponent
@@ -52,17 +68,21 @@ fn test_policy_subnormal_flushing() {
5268
let s3 = F32Scalar::new(neg_sub);
5369
assert_eq!(s3.to_f32().to_bits(), 0, "Negative subnormal not flushed to +0.0");
5470
}
71+
*/
5572

5673
#[test]
5774
#[cfg(feature = "serde")]
5875
fn test_policy_serialization_guard() {
5976
// Manually construct JSON with -0.0
60-
let json = r#"{ "value": -0.0 }"#;
77+
let json = r#"-0.0"#;
6178

6279
// If Deserialize is derived, this will put -0.0 into the struct, violating the invariant.
6380
// If implemented manually via new(), it should be +0.0.
6481
let s: F32Scalar = serde_json::from_str(json).expect("Failed to deserialize");
6582

66-
assert_eq!(s.to_f32().to_bits(), 0, "Deserialized -0.0 was not canonicalized!");
83+
assert_eq!(
84+
s.to_f32().to_bits(),
85+
0,
86+
"Deserialized -0.0 was not canonicalized!"
87+
);
6788
}
68-
*/

crates/rmg-core/tests/math_validation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@
66
//! Ensures scalar, vector, matrix, quaternion, and PRNG behaviour stays
77
//! consistent with the documented fixtures across platforms.
88
9-
use once_cell::sync::Lazy;
109
use serde::Deserialize;
10+
use std::sync::LazyLock;
1111

1212
use rmg_core::math::{self, Mat4, Prng, Quat, Vec3};
1313

1414
const FIXTURE_PATH: &str = "crates/rmg-core/tests/fixtures/math-fixtures.json";
1515
static RAW_FIXTURES: &str = include_str!("fixtures/math-fixtures.json");
1616

1717
#[allow(clippy::expect_fun_call)]
18-
static FIXTURES: Lazy<MathFixtures> = Lazy::new(|| {
18+
static FIXTURES: LazyLock<MathFixtures> = LazyLock::new(|| {
1919
let fixtures: MathFixtures = {
2020
#[allow(clippy::expect_fun_call)]
2121
{
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots>
3+
4+
//! Comprehensive tests for NaN canonicalization and determinism.
5+
//!
6+
//! This test suite validates that `F32Scalar` correctly implements the strict
7+
//! determinism policy by:
8+
//! 1. Canonicalizing all NaN bit patterns (signaling, quiet, payload) to a
9+
//! single standard Positive Quiet NaN (`0x7fc00000`).
10+
//! 2. Preserving Infinities (distinct from NaNs).
11+
//! 3. Ensuring reflexivity (`x == x`) holds for the canonicalized NaNs.
12+
13+
use rmg_core::math::scalar::F32Scalar;
14+
use rmg_core::math::Scalar;
15+
16+
/// Verifies that various classes of NaN values are correctly canonicalized.
17+
///
18+
/// This includes:
19+
/// - Specific edge cases (smallest/largest mantissas).
20+
/// - Signaling vs Quiet NaNs.
21+
/// - Positive vs Negative NaNs.
22+
/// - NaNs with arbitrary payloads.
23+
///
24+
/// It also performs a sweep of low and high mantissa bits to catch potential
25+
/// off-by-one errors in bitmask logic.
26+
#[test]
27+
fn test_comprehensive_nan_coverage() {
28+
// IEEE 754 float32:
29+
// Sign: 1 bit (31)
30+
// Exponent: 8 bits (23-30) -> All 1s for NaN/Inf (0xFF)
31+
// Mantissa: 23 bits (0-22) -> Non-zero for NaN (Zero for Inf)
32+
33+
let exponent_mask = 0x7F800000;
34+
let mantissa_mask = 0x007FFFFF;
35+
let sign_mask = 0x80000000u32;
36+
37+
// 1. Verify specific edge case NaNs
38+
let patterns = vec![
39+
0x7F800001, // Smallest mantissa, positive
40+
0x7FFFFFFF, // Largest mantissa, positive
41+
0xFF800001, // Smallest mantissa, negative
42+
0xFFFFFFFF, // Largest mantissa, negative
43+
0x7FC00000, // Canonical qNaN positive
44+
0xFFC00000, // Canonical qNaN negative
45+
0x7FA00000, // Some payload
46+
0x7F80DEAD, // Dead beef payload
47+
];
48+
49+
for bits in patterns {
50+
let f = f32::from_bits(bits);
51+
// Pre-condition: verify our assumption that these ARE NaNs according to Rust
52+
assert!(f.is_nan(), "Rust did not identify {:#x} as NaN", bits);
53+
54+
let scalar = F32Scalar::new(f);
55+
let out_bits = scalar.to_f32().to_bits();
56+
57+
assert_eq!(
58+
out_bits, 0x7fc00000,
59+
"Input NaN {:#x} was not canonicalized to 0x7fc00000, got {:#x}",
60+
bits, out_bits
61+
);
62+
63+
// Explicitly test reflexivity for the canonicalized NaN
64+
assert_eq!(
65+
scalar, scalar,
66+
"Reflexivity failed for canonicalized NaN from input {:#x}",
67+
bits
68+
);
69+
assert_eq!(
70+
scalar.cmp(&scalar),
71+
std::cmp::Ordering::Equal,
72+
"Ordering reflexivity failed for canonicalized NaN from input {:#x}",
73+
bits
74+
);
75+
}
76+
77+
// 2. Fuzz / Sweep a range of mantissas
78+
// We can't check all 2^23 * 2 NaNs, but we can check a lot.
79+
// Let's check the first 1000 and last 1000 mantissas for both signs.
80+
81+
let signs = [0u32, sign_mask];
82+
83+
for sign in signs {
84+
// Mantissa cannot be 0 (that's Infinity)
85+
// Loop 1..1000
86+
for m in 1..1000 {
87+
let bits = sign | exponent_mask | m;
88+
let f = f32::from_bits(bits);
89+
let s = F32Scalar::new(f);
90+
assert_eq!(
91+
s.to_f32().to_bits(),
92+
0x7fc00000,
93+
"Failed low mantissa {:#x}",
94+
bits
95+
);
96+
}
97+
// Loop max-1000..max
98+
for m in (mantissa_mask - 1000)..=mantissa_mask {
99+
let bits = sign | exponent_mask | m;
100+
let f = f32::from_bits(bits);
101+
let s = F32Scalar::new(f);
102+
assert_eq!(
103+
s.to_f32().to_bits(),
104+
0x7fc00000,
105+
"Failed high mantissa {:#x}",
106+
bits
107+
);
108+
}
109+
}
110+
}
111+
112+
/// Verifies that Infinity values are preserved and NOT canonicalized.
113+
///
114+
/// The determinism policy requires finite numbers and infinities to be preserved
115+
/// (modulo -0.0 normalization), while only NaNs are collapsed.
116+
#[test]
117+
fn test_infinity_preservation() {
118+
// Ensure we didn't accidentally canonicalize Infinity
119+
let pos_inf = f32::from_bits(0x7F800000);
120+
let neg_inf = f32::from_bits(0xFF800000);
121+
122+
assert!(!pos_inf.is_nan());
123+
assert!(!neg_inf.is_nan());
124+
125+
let s_pos = F32Scalar::new(pos_inf);
126+
let s_neg = F32Scalar::new(neg_inf);
127+
128+
assert_eq!(s_pos.to_f32().to_bits(), 0x7F800000);
129+
assert_eq!(s_neg.to_f32().to_bits(), 0xFF800000);
130+
}

docs/decision-log.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
| ---- | ------- | -------- | --------- | ----------- |
99
| 2025-11-29 | LICENSE | Add SPDX headers to all files | LEGAL PROTECTION 🛡️✨ |
1010
| 2025-11-30 | `F32Scalar` NaN reflexivity | Update `PartialEq` implementation to use `total_cmp` (via `Ord`) instead of `f32::eq`. | Ensures `Eq` reflexivity holds even for NaN (`NaN == NaN`), consistent with `Ord`. Prevents violations of the `Eq` contract in collections. | `F32Scalar` now behaves as a totally ordered type; NaN values are considered equal to themselves and comparable. |
11+
| 2025-11-30 | `F32Scalar` NaN canonicalization | Canonicalize all NaNs to `0x7fc00000` in `F32Scalar::new`. Update MSRV to 1.83.0 for `const` `is_nan`/`from_bits`. | Guarantees bit-level determinism for all float values including error states. Unifies representation across platforms. MSRV bump enables safe, readable `const` implementation. | All NaNs are now bitwise identical; `const fn` constructors remain available; `rmg-core` requires Rust 1.83.0+. |
1112
| 2025-11-30 | `F32Scalar` canonicalization | Enforce bitwise determinism by canonicalizing `-0.0` to `+0.0` for all `F32Scalar` instances; implement `PartialEq`, `Eq`, `PartialOrd`, `Ord`, `Display`. Make `value` field private. | Essential for bit-perfect cross-platform determinism in math operations and comparisons, especially for hashing and serialization. Prevents accidental introduction of `-0.0` by direct field access. | Guarantees consistent numerical behavior for `F32Scalar`; all public API methods and constructors now ensure canonical zero. |
1213
| 2025-11-29 | `F32Scalar` | Add `rmg-core::math::scalar::F32Scalar` type | Now we have it. |
1314
| 2025-11-03 | Scalar foundation | Add `rmg-core::math::Scalar` trait (operator supertraits + sin/cos) | Arithmetic via `Add/Sub/Mul/Div/Neg` supertraits for ergonomic `+ - * /`; `sin/cos` methods declared; canonicalization/LUTs deferred | Unblocks F32Scalar and DFix64 implementations; math code can target a stable trait |

docs/execution-plan.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,13 @@ This is Codex’s working map for building Echo. Update it relentlessly—each s
8080
- Expected behavior: identical drain order and semantics; minor memory increase for counts on 64‑bit.
8181
- Next: run full workspace Clippy + tests, then commit.
8282
- CI follow-up: add `PortSet::iter()` (additive API) to satisfy scheduler iteration on GH runners.
83-
> 2025-11-30 – F32Scalar canonicalization and trait implementations
83+
> 2025-11-30 – F32Scalar canonicalization and trait implementations (COMPLETED)
8484
8585
- Goal: Ensure bit-level deterministic handling of zero for `F32Scalar` and implement necessary traits for comprehensive numerical behavior.
8686
- Scope: `crates/rmg-core/src/math/scalar.rs` and `crates/rmg-core/tests/math_scalar_tests.rs`.
8787
- Changes:
8888
- `F32Scalar` canonicalizes `-0.0` to `+0.0` on construction.
89+
- `F32Scalar` canonicalizes all NaNs to `0x7fc00000` on construction (new).
8990
- `value` field made private.
9091
- `PartialEq` implemented via `Ord` (total_cmp) to ensure `NaN == NaN` (reflexivity).
9192
- `Eq`, `PartialOrd`, `Ord`, `Display` traits implemented.

0 commit comments

Comments
 (0)