Repository: sharkdp/numbat (rust)

Issue:
Unclear assertion errors due to floating point numbers
I recently encountered a an error message that asserted that two identically printed values were not the same. I know this is due to floating point numbers being imperfect representations, but from a user perspective I thought we might want to point it out to the user that this is probably what went wrong.

I would suggest that we check for the two printed values, and if they are the exact same, we point out that floating point imprecision might be the cause. This will especially be helpful for less technical people so they don't assume it's a bug in numbat.

See an example [here](https://numbat.dev/?q=135+degree+->+rad⏎2%2F3+rad+->+deg⏎1%2B2%2F3+rad+->+deg⏎1%2B5%2F6+rad+->+deg⏎1%2B9%2F10+rad+->+deg⏎1+rad+->+deg⏎2+rad+->+deg⏎2+radian+->+deg⏎2+pi+->+deg⏎1+pi+->+deg⏎2%2F3+pi+->+deg⏎%282%2F3%29pi⏎3+pi+%2F+2⏎%283+pi%29+%2F+2⏎%283+pi%29+%2F+2+->+deg⏎2%2F3+pi+->+deg⏎%283+pi%29+%2F+2+->+deg⏎3%2F2+pi+->+deg⏎2+pi+%2F+3+->+deg⏎4+pi+%2F+5+->+deg⏎5+pi+%2F+6+->+deg⏎5+pi+%2F+7+->+deg⏎6+pi+%2F+8+->+deg⏎3+pi+%2F+4+->+deg⏎4⏎sqrt%28444.398^2+-+%28444.398+-+266.783%29^2%29+-+1180.532⏎sqrt%28400^2+-+%28444.398+-+266.783%29^2%29+-+1180.532⏎sqrt%28%28-822.129+-+1180.53%29^2+%2B+%28444.398+-+266.783%29^2%29⏎sqrt%28400^2+-+%28444.398+-+266.783%29^2%29+%2B+1180.532⏎sqrt%28%281538.94+-+1180.53%29^2+%2B+%28444.398+-+266.783%29^2%29⏎0+%2F+10⏎0.01+%2F+10⏎10+%2F+10⏎10+%2F+10^2⏎20+%2F+20^2⏎2+%2F+2^2⏎2^2+%2F+2⏎20^2+%2F+20⏎20^1.5+%2F+20⏎2^1.5+%2F+2⏎2^1.1+%2F+2⏎2^1+%2F+2⏎2+%2F+2^0.5⏎20+%2F+20^0.5⏎200+%2F+200^0.5⏎200+%2F+200^0.25⏎200+%2F+200^0.75⏎20+%2F+20^0.75⏎2+%2F+2^0.75⏎2+%2F+2^0.95⏎2+%2F+2!⏎20+%2F+20!⏎20!+%2F+20⏎2!+%2F+2⏎2^2+%2F+2^3⏎20^2+%2F+20^3⏎20+%2F+20⏎200+%2F+200⏎2+%2F+2⏎%28200%2B200%29%2F200⏎%2820%2B20%29%2F20⏎%282%2B2%29%2F2⏎%282%2B2%29%2F2^2⏎%2820%2B20%29%2F20^20⏎%2820%2B20%29%2F20^2⏎%2820%2B20%29%2F20^0.5⏎%282%2B2%29%2F2^0.5⏎%28200%2B200%29%2F200^0.5⏎%28200%2B200%29%2F200^%281%2F3%29⏎%2820%2B20%29%2F20^%281%2F3%29⏎%2820%2B20%29%2F20^%281%2F2%29⏎%2820%2B20%29%2F20^%283%2F4%29⏎%282%2B2%29%2F2^%283%2F4%29⏎%282%2B2%29%2F2^2⏎%2820%2B20%29%2F2^2⏎%2820%2B20%29%2F20^2⏎20^2+%2F+%2820%2B20%29⏎2^2+%2F+%282%2B2%29⏎200^2+%2F+%28200%2B200%29⏎200^2+%2F+%28200%2B10%29⏎200^2+%2F+%28200%2B1000%29⏎2^2+%2F+%282%2B100%29⏎2^2+%2F+%282%2B10%29⏎2^2+%2F+%282%2B1%29⏎1^2+%2F+%282%2B1%29⏎1^2+%2F+%281%2B1%29⏎10^2+%2F+%2810%2B10%29⏎1^2+%2F+%281%2B10%29⏎100^2+%2F+%28100%2B1000%29⏎900^2+%2F+%28900%2B1000%29⏎800^2+%2F+%28800%2B1000%29⏎700^2+%2F+%28700%2B1000%29⏎750^2+%2F+%28750%2B1000%29⏎750^3+%2F+%28750%2B1000%29⏎750^2+%2F+%28750%2B1000%29^2⏎100^2+%2F+%28100%2B100%29^2⏎99^2+%2F+%2899%2B100%29^2⏎99.5^2+%2F+%2899.5%2B100%29^2⏎99.75^2+%2F+%2899.75%2B100%29^2⏎99.76^2+%2F+%2899.76%2B100%29^2⏎100%2F1000⏎200%2F1000⏎0%2F1000⏎1%2F1000⏎0.01%2F1000⏎0.001%2F1000⏎0.0001%2F1000⏎100%2F1000⏎1+MB+->+B+*+37⏎1+MB+->+37+B⏎fn+_zero_length<A%3A+Dim>%28val%3A+A%29+->+A+%3D+val+*+0+->+val⏎fn+_mixed_unit_list<A%3A+Dim>%28val%3A+A%2C+units%3A+List<A>%2C+min_unit%3A+A%2C+acc%3A+List<A>%29+->+List<A>+%3D%0A++if+trunc_in%28min_unit%2C+val%29+%3D%3D+0%0A++then+concat%28acc%2C+map%28_zero_length%2C+units%29%29%0A++else+if+len%28units%29+%3D%3D+1%0A++then+cons_end%28val+->+head%28units%29%2C+acc%29%0A++else+_mixed_unit_list%28val+-+unit_val%2C+tail%28units%29%2C+min_unit%2C+cons_end%28unit_val%2C+acc%29%29%0A++where+unit_val%3A+A+%3D%0A++++if+%28len%28units%29+>+0%29%0A++++then+%28val+|>+round_in%28min_unit%29+|>+trunc_in%28head%28units%29%29%29%0A++++else+error%28"Units+list+cannot+be+empty"%29⏎fn+unit_list<A%3A+Dim>%28units%3A+List<A>%2C+value%3A+A%29+->+List<A>+%3D+_mixed_unit_list%28value%2C+units%2C+minimum%28units%29+%2F+100%2C+%5B%5D%29⏎assert_eq%28%5B1+deg%2C+23+arcmin%2C+4+arcsec%5D%2C+%5B1+deg%2C+23+arcmin%2C+4+arcsec%5D%29⏎assert_eq%28unit_list%28%5Bhour%5D%2C+1.5+hour%29%2C+%5B1.5+hour%5D%29⏎assert_eq%28unit_list%28%5Bhour%2C+minutes%5D%2C+1.5+hour%29%2C+%5B1+hour%2C+30+minutes%5D%29⏎fn+_zero_length<A%3A+Dim>%28val%3A+A%29+->+A+%3D+val+*+0+->+val⏎fn+_mixed_unit_list<D%3A+Dim>%28val%3A+D%2C+units%3A+List<D>%2C+acc%3A+List<D>%29+->+List<D>+%3D%0A++if+val+%3D%3D+0%0A++++then+concat%28acc%2C+map%28_zero_length%2C+units%29%29%0A++++else+if+len%28units%29+%3D%3D+1%0A++++++then+cons_end%28val+->+head%28units%29%2C+acc%29%0A++++++else+_mixed_unit_list%28val+-+unit_val%2C+tail%28units%29%2C+cons_end%28unit_val%2C+acc%29%29%0A++where+unit_val%3A+D+%3D%0A++++if+%28len%28units%29+>+0%29%0A++++++then+%28val+|>+trunc_in%28head%28units%29%29%29%0A++++++else+error%28"Units+list+cannot+be+empty"%29⏎fn+unit_list<D%3A+Dim>%28units%3A+List<D>%2C+value%3A+D%29+->+List<D>+%3D+_mixed_unit_list%28value%2C+units%2C+%5B%5D%29⏎let+test1+%3D+12+m+%2B+34+cm+%2B+5+mm+%2B+678+µm⏎assert_eq%28test1+|>+unit_list%28%5Bm%5D%29+|>+head%2C+test1%29⏎assert_eq%28test1+|>+unit_list%28%5Bm,+cm%5D%29,+%5B12+m,+34.5678+cm%5D%29⏎)

![image](https://github.com/user-attachments/assets/0c26cf23-93d5-4eed-b105-3a522ab56df3)


Pull request description:
Add note regarding floating point errors to assert_eq
This PR resolves #567 and adds a note to `assert_eq` errors if the two printed values are exactly the same to hint the user that this may be a floating point precision problem.


Patch:
diff --git a/numbat/src/diagnostic.rs b/numbat/src/diagnostic.rs
index 85f1757..ca6750d 100644
--- a/numbat/src/diagnostic.rs
+++ b/numbat/src/diagnostic.rs
@@ -500,16 +500,18 @@ impl ErrorDiagnostic for RuntimeError {
                 .with_labels(vec![span
                     .diagnostic_label(LabelStyle::Primary)
                     .with_message("assertion failed")])],
-            RuntimeError::AssertEq2Failed(span_lhs, lhs, span_rhs, rhs) => {
+            RuntimeError::AssertEq2Failed(assert_eq2_error) => {
                 vec![Diagnostic::error()
                     .with_message("Assertion failed")
                     .with_labels(vec![
-                        span_lhs
+                        assert_eq2_error
+                            .span_lhs
                             .diagnostic_label(LabelStyle::Secondary)
-                            .with_message(format!("{lhs}")),
-                        span_rhs
+                            .with_message(format!("{}", assert_eq2_error.lhs)),
+                        assert_eq2_error
+                            .span_rhs
                             .diagnostic_label(LabelStyle::Primary)
-                            .with_message(format!("{rhs}")),
+                            .with_message(format!("{}", assert_eq2_error.rhs)),
                     ])
                     .with_notes(vec![inner])]
             }
diff --git a/numbat/src/ffi/procedures.rs b/numbat/src/ffi/procedures.rs
index 04d1880..8d04cd5 100644
--- a/numbat/src/ffi/procedures.rs
+++ b/numbat/src/ffi/procedures.rs
@@ -4,8 +4,14 @@ use std::sync::OnceLock;
 
 use super::macros::*;
 use crate::{
-    ast::ProcedureKind, ffi::ControlFlow, interpreter::assert_eq_3::AssertEq3Error,
-    pretty_print::PrettyPrint, span::Span, value::Value, vm::ExecutionContext, RuntimeError,
+    ast::ProcedureKind,
+    ffi::ControlFlow,
+    interpreter::assert_eq::{AssertEq2Error, AssertEq3Error},
+    pretty_print::PrettyPrint,
+    span::Span,
+    value::Value,
+    vm::ExecutionContext,
+    RuntimeError,
 };
 
 use super::{Args, Callable, ForeignFunction};
@@ -81,12 +87,12 @@ fn assert_eq(_: &mut ExecutionContext, mut args: Args, arg_spans: Vec<Span>) ->
         let lhs = arg!(args);
         let rhs = arg!(args);
 
-        let error = ControlFlow::Break(RuntimeError::AssertEq2Failed(
-            span_lhs,
-            lhs.clone(),
-            span_rhs,
-            rhs.clone(),
-        ));
+        let error = ControlFlow::Break(RuntimeError::AssertEq2Failed(AssertEq2Error {
+            span_lhs: span_lhs,
+            lhs: lhs.clone(),
+            span_rhs: span_rhs,
+            rhs: rhs.clone(),
+        }));
 
         if lhs.is_quantity() {
             let lhs = lhs.unsafe_as_quantity();
diff --git a/numbat/src/interpreter/assert_eq_3.rs b/numbat/src/interpreter/assert_eq.rs
similarity index 89%
rename from numbat/src/interpreter/assert_eq_3.rs
rename to numbat/src/interpreter/assert_eq.rs
index 8a2df21..b3b879a 100644
--- a/numbat/src/interpreter/assert_eq_3.rs
+++ b/numbat/src/interpreter/assert_eq.rs
@@ -1,8 +1,33 @@
-use crate::{quantity::Quantity, span::Span};
+use crate::{quantity::Quantity, span::Span, value::Value};
 use compact_str::{format_compact, CompactString};
 use std::fmt::Display;
 use thiserror::Error;
 
+#[derive(Debug, Clone, Error, PartialEq, Eq)]
+pub struct AssertEq2Error {
+    pub span_lhs: Span,
+    pub lhs: Value,
+    pub span_rhs: Span,
+    pub rhs: Value,
+}
+
+impl Display for AssertEq2Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let optional_message = if format!("{}", self.lhs) == format!("{}", self.rhs) {
+            "\nNote: The two printed values appear to be the same, this may be due to floating point precision errors.\n      \
+            For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε)."
+        } else {
+            ""
+        };
+
+        write!(
+            f,
+            "Assertion failed because the following two values are not the same:\n  {}\n  {}{}",
+            self.lhs, self.rhs, optional_message
+        )
+    }
+}
+
 #[derive(Debug, Clone, Error, PartialEq, Eq)]
 pub struct AssertEq3Error {
     pub span_lhs: Span,
diff --git a/numbat/src/interpreter/mod.rs b/numbat/src/interpreter/mod.rs
index 2082874..fd74dd6 100644
--- a/numbat/src/interpreter/mod.rs
+++ b/numbat/src/interpreter/mod.rs
@@ -1,4 +1,4 @@
-pub(crate) mod assert_eq_3;
+pub(crate) mod assert_eq;
 
 use crate::{
     dimension::DimensionRegistry,
@@ -12,7 +12,7 @@ use crate::{
 
 pub use crate::markup as m;
 
-use assert_eq_3::AssertEq3Error;
+use assert_eq::{AssertEq2Error, AssertEq3Error};
 use compact_str::{CompactString, ToCompactString};
 use thiserror::Error;
 
@@ -32,8 +32,8 @@ pub enum RuntimeError {
     QuantityError(QuantityError),
     #[error("Assertion failed")]
     AssertFailed(Span),
-    #[error("Assertion failed because the following two values are not the same:\n  {1}\n  {3}")]
-    AssertEq2Failed(Span, Value, Span, Value),
+    #[error("{0}")]
+    AssertEq2Failed(AssertEq2Error),
     #[error("{0}")]
     AssertEq3Failed(AssertEq3Error),
     #[error("Could not load exchange rates from European Central Bank.")]


Test patch:
diff --git a/numbat/tests/interpreter.rs b/numbat/tests/interpreter.rs
index 33a1d4e..94816ee 100644
--- a/numbat/tests/interpreter.rs
+++ b/numbat/tests/interpreter.rs
@@ -856,7 +856,7 @@ fn test_statement_pretty_printing() {
 mod tests {
     use super::*;
     #[cfg(test)]
-    mod assert_eq_3 {
+    mod assert_eq {
         use super::*;
 
         #[test]
@@ -960,5 +960,21 @@ mod tests {
               -77.0089°
             "###);
         }
+
+        #[test]
+        fn test_floating_point_warning() {
+            insta::assert_snapshot!(fail("assert_eq(2+ 2, 2 + 1)"), @r###"
+            Assertion failed because the following two values are not the same:
+              4
+              3
+            "###);
+            insta::assert_snapshot!(fail("assert_eq(2 + 2e-12, 2 + 1e-12)"), @r###"
+            Assertion failed because the following two values are not the same:
+              2.0
+              2.0
+            Note: The two printed values appear to be the same, this may be due to floating point precision errors.
+                  For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε).
+            "###);
+        }
     }
 }