Browse Source

feat(ffi/procedures): assert_eq/3: better error message using consistent units for comparands

* convert comparands to the units of epsilon before comparison
* show original units where possible in parentheses
Manish Bhasin 1 year ago
parent
commit
cd65049596

+ 3 - 1
book/src/procedures.md

@@ -47,7 +47,9 @@ assert_eq(q1, q2, ε)
 ```
 
 where the first version tests for exact equality while the second version tests for approximate
-equality \\( |q_1-q_2| < \epsilon \\) with a specified accuracy of \\( \epsilon \\). For example:
+equality \\( |q_1-q_2| < \epsilon \\) with a specified accuracy of \\( \epsilon \\).
+Note that the input quantities are converted to the units of \\( \epsilon \\) before comparison.
+For example:
 
 ```nbt
 assert_eq(2 + 3, 5)

+ 2 - 0
examples/runtime_error/assert_eq_3.nbt

@@ -0,0 +1,2 @@
+# fail approximate equality test with epsilon in units that are different from either comparand
+assert_eq(2m, 201cm, 0.1cm to mm)

+ 12 - 3
numbat/src/diagnostic.rs

@@ -518,16 +518,25 @@ impl ErrorDiagnostic for RuntimeError {
                     ])
                     .with_notes(vec![inner])]
             }
-            RuntimeError::AssertEq3Failed(span_lhs, lhs, span_rhs, rhs, _) => {
+            RuntimeError::AssertEq3Failed(
+                span_lhs,
+                lhs_original,
+                lhs_converted,
+                span_rhs,
+                rhs_original,
+                rhs_converted,
+                _eps,
+                _diff_abs,
+            ) => {
                 vec![Diagnostic::error()
                     .with_message("Assertion failed")
                     .with_labels(vec![
                         span_lhs
                             .diagnostic_label(LabelStyle::Secondary)
-                            .with_message(format!("{lhs}")),
+                            .with_message(format!("{lhs_converted} ({lhs_original})")),
                         span_rhs
                             .diagnostic_label(LabelStyle::Primary)
-                            .with_message(format!("{rhs}")),
+                            .with_message(format!("{rhs_converted} ({rhs_original})")),
                     ])
                     .with_notes(vec![format!("{self:#}")])]
             }

+ 34 - 21
numbat/src/ffi/procedures.rs

@@ -4,8 +4,8 @@ use std::sync::OnceLock;
 
 use super::macros::*;
 use crate::{
-    ast::ProcedureKind, ffi::ControlFlow, pretty_print::PrettyPrint, span::Span, value::Value,
-    vm::ExecutionContext, RuntimeError,
+    ast::ProcedureKind, ffi::ControlFlow, pretty_print::PrettyPrint, quantity::Quantity,
+    span::Span, value::Value, vm::ExecutionContext, RuntimeError,
 };
 
 use super::{Args, Callable, ForeignFunction};
@@ -107,29 +107,42 @@ fn assert_eq(_: &mut ExecutionContext, mut args: Args, arg_spans: Vec<Span>) ->
             error
         }
     } else {
-        let lhs = quantity_arg!(args);
-        let rhs = quantity_arg!(args);
-        let result = &lhs - &rhs;
+        let lhs_original = quantity_arg!(args);
+        let rhs_original = quantity_arg!(args);
         let eps = quantity_arg!(args);
 
+        let lhs_converted = lhs_original.convert_to(eps.unit());
+        let lhs_converted = match lhs_converted {
+            Err(e) => return ControlFlow::Break(RuntimeError::QuantityError(e)),
+            Ok(q) => q,
+        };
+        let rhs_converted = rhs_original.convert_to(eps.unit());
+        let rhs_converted = match rhs_converted {
+            Err(e) => return ControlFlow::Break(RuntimeError::QuantityError(e)),
+            Ok(q) => q,
+        };
+
+        let result = &lhs_converted - &rhs_converted;
+
         match result {
-            Ok(diff) => match diff.convert_to(eps.unit()) {
-                Err(e) => ControlFlow::Break(RuntimeError::QuantityError(e)),
-                Ok(diff_converted) => {
-                    if diff_converted.unsafe_value().to_f64().abs() < eps.unsafe_value().to_f64() {
-                        ControlFlow::Continue(())
-                    } else {
-                        ControlFlow::Break(RuntimeError::AssertEq3Failed(
-                            span_lhs,
-                            lhs.clone(),
-                            span_rhs,
-                            rhs.clone(),
-                            eps.clone(),
-                        ))
-                    }
-                }
-            },
             Err(e) => ControlFlow::Break(RuntimeError::QuantityError(e)),
+            Ok(diff) => {
+                let diff_abs = diff.unsafe_value().to_f64().abs();
+                if diff_abs < eps.unsafe_value().to_f64() {
+                    ControlFlow::Continue(())
+                } else {
+                    ControlFlow::Break(RuntimeError::AssertEq3Failed(
+                        span_lhs,
+                        lhs_original.clone(),
+                        lhs_converted.clone(),
+                        span_rhs,
+                        rhs_original.clone(),
+                        rhs_converted.clone(),
+                        eps.clone(),
+                        Quantity::new_f64(diff_abs, diff.unit().clone()),
+                    ))
+                }
+            }
         }
     }
 }

+ 11 - 2
numbat/src/interpreter.rs

@@ -30,8 +30,17 @@ pub enum RuntimeError {
     AssertFailed(Span),
     #[error("Assertion failed because the following two values are not the same:\n  {1}\n  {3}")]
     AssertEq2Failed(Span, Value, Span, Value),
-    #[error("Assertion failed because the following two quantities differ by more than {4}:\n  {1}\n  {3}")]
-    AssertEq3Failed(Span, Quantity, Span, Quantity, Quantity),
+    #[error("Assertion failed because the following two quantities differ by {7}, which is more than or equal to {6}:\n  {2} ({1})\n  {5} ({4})")]
+    AssertEq3Failed(
+        Span,
+        Quantity,
+        Quantity,
+        Span,
+        Quantity,
+        Quantity,
+        Quantity,
+        Quantity,
+    ),
     #[error("Could not load exchange rates from European Central Bank.")]
     CouldNotLoadExchangeRates,
     #[error("User error: {0}")]