From f16aac2ea0d64b0eb284ccf5a24a145e3d6e3c32 Mon Sep 17 00:00:00 2001 From: Alex Ivliev Date: Fri, 10 Jan 2025 15:25:31 +0100 Subject: [PATCH 1/3] exclude infinity from the floating point wrappers --- nemo-physical/src/datatypes.rs | 3 - nemo-physical/src/datatypes/double.rs | 58 +++++++++++-------- nemo-physical/src/datatypes/float.rs | 63 ++++++++++++--------- nemo-physical/src/datatypes/float_is_nan.rs | 18 ------ nemo-physical/src/error.rs | 6 +- 5 files changed, 73 insertions(+), 75 deletions(-) delete mode 100644 nemo-physical/src/datatypes/float_is_nan.rs diff --git a/nemo-physical/src/datatypes.rs b/nemo-physical/src/datatypes.rs index 744368d45..d4fd326bd 100644 --- a/nemo-physical/src/datatypes.rs +++ b/nemo-physical/src/datatypes.rs @@ -12,9 +12,6 @@ pub use double::Double; /// Module for defining [Float] pub mod float; pub use float::Float; -/// Module for defining [FloatIsNaN] -pub(crate) mod float_is_nan; -pub(crate) use float_is_nan::FloatIsNaN; /// Module for defining [Ring] pub(crate) mod ring; pub(crate) use ring::Ring; diff --git a/nemo-physical/src/datatypes/double.rs b/nemo-physical/src/datatypes/double.rs index 78c8a1c98..a422254c6 100644 --- a/nemo-physical/src/datatypes/double.rs +++ b/nemo-physical/src/datatypes/double.rs @@ -1,13 +1,23 @@ -use super::run_length_encodable::FloatingStep; -use super::{FloatIsNaN, FloorToUsize, RunLengthEncodable}; -use crate::error::{Error, ReadingError}; -use crate::function::definitions::numeric::traits::{CheckedPow, CheckedSquareRoot}; -use num::traits::CheckedNeg; -use num::{Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, FromPrimitive, One, Zero}; -use std::cmp::Ordering; -use std::fmt; -use std::iter::{Product, Sum}; -use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Rem, Sub, SubAssign}; +//! This module defines a wrapper type [Double] for [f64] that excludes NaN and infinity. + +use std::{ + cmp::Ordering, + fmt, + iter::{Product, Sum}, + ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Rem, Sub, SubAssign}, +}; + +use num::{ + traits::CheckedNeg, Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, FromPrimitive, + One, Zero, +}; + +use crate::{ + error::{Error, ReadingError}, + function::definitions::numeric::traits::{CheckedPow, CheckedSquareRoot}, +}; + +use super::{run_length_encodable::FloatingStep, FloorToUsize, RunLengthEncodable}; #[cfg(test)] use quickcheck::{Arbitrary, Gen}; @@ -20,22 +30,22 @@ impl Double { /// Wraps the given [f64]-`value` as a value over [Double]. /// /// # Errors - /// The given `value` is [f64::NAN]. + /// Returns an error if `value` is [f32::NAN] or infinite. pub fn new(value: f64) -> Result { - if value.is_nan() { - return Err(FloatIsNaN.into()); + if !value.is_finite() { + return Err(ReadingError::InvalidFloat.into()); } Ok(Self(value)) } - /// Wraps the given [f64]-`value`, that is a number, as a value over [Double]. + /// Wraps the given [f64]-`value` as a value over [Double]. /// /// # Panics - /// The given `value` is [f64::NAN]. + /// Panics if `value` is [f64::NAN] or not finite. pub fn from_number(value: f64) -> Self { if value.is_nan() { - panic!("The provided value is not a number (NaN)!") + panic!("floating point values must be finite") } Self(value) @@ -43,7 +53,7 @@ impl Double { /// Computes the absolute value. pub(crate) fn abs(self) -> Self { - Double::new(self.0.abs()).expect("Taking the absolute value cannot result in NaN") + Double::new(self.0.abs()).expect("operation returned in invalid float") } /// Returns the logarithm of the number with respect to an arbitrary base. @@ -53,34 +63,34 @@ impl Double { /// Computes the sine of a number (in radians). pub(crate) fn sin(self) -> Self { - Double::new(self.0.sin()).expect("Operation does not result in NaN") + Double::new(self.0.sin()).expect("operation returned in invalid float") } /// Computes the cosine of a number (in radians). pub(crate) fn cos(self) -> Self { - Double::new(self.0.cos()).expect("Operation does not result in NaN") + Double::new(self.0.cos()).expect("operation returned in invalid float") } /// Computes the tangent of a number (in radians). pub(crate) fn tan(self) -> Self { - Double::new(self.0.tan()).expect("Operation does not result in NaN") + Double::new(self.0.tan()).expect("operation returned in invalid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn round(self) -> Self { - Double::new(self.0.round()).expect("Operation does not result in NaN") + Double::new(self.0.round()).expect("operation returned in invalid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn ceil(self) -> Self { - Double::new(self.0.ceil()).expect("Operation does not result in NaN") + Double::new(self.0.ceil()).expect("operation returned in invalid float") } /// Returns the largest integer less than or equal to `self`. pub(crate) fn floor(self) -> Self { - Double::new(self.0.floor()).expect("Operation does not result in NaN") + Double::new(self.0.floor()).expect("operation returned in invalid float") } } @@ -294,7 +304,7 @@ impl Bounded for Double { impl Arbitrary for Double { fn arbitrary(g: &mut Gen) -> Self { let mut value = f64::arbitrary(g); - while value.is_nan() { + while !value.is_finite() { value = f64::arbitrary(g); } diff --git a/nemo-physical/src/datatypes/float.rs b/nemo-physical/src/datatypes/float.rs index f5908131a..1e1a48c22 100644 --- a/nemo-physical/src/datatypes/float.rs +++ b/nemo-physical/src/datatypes/float.rs @@ -1,18 +1,27 @@ -use super::run_length_encodable::FloatingStep; -use super::{FloatIsNaN, FloorToUsize, RunLengthEncodable}; -use crate::error::Error; -use crate::function::definitions::numeric::traits::{CheckedPow, CheckedSquareRoot}; -use num::traits::CheckedNeg; -use num::{Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, FromPrimitive, One, Zero}; -use std::cmp::Ordering; -use std::fmt; -use std::iter::{Product, Sum}; -use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Rem, Sub, SubAssign}; +//! This module defines a wrapper type [Float] for [f32] that excludes NaN and infinity. + +use std::{ + cmp::Ordering, + fmt, + iter::{Product, Sum}, + ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Rem, Sub, SubAssign}, +}; + +use num::{ + traits::CheckedNeg, Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, FromPrimitive, + One, Zero, +}; + +use super::{run_length_encodable::FloatingStep, FloorToUsize, RunLengthEncodable}; +use crate::{ + error::{Error, ReadingError}, + function::definitions::numeric::traits::{CheckedPow, CheckedSquareRoot}, +}; #[cfg(test)] use quickcheck::{Arbitrary, Gen}; -/// Wrapper for [f32`] that does not allow [`f32::NAN] values. +/// Wrapper for [f32] that does not allow [f32::NAN] values. #[derive(Copy, Clone, Debug, PartialEq, Default)] pub struct Float(f32); @@ -20,22 +29,22 @@ impl Float { /// Wraps the given [f32]-`value` as a value over [Float]. /// /// # Errors - /// The given `value` is [f32::NAN]. + /// Returns an error if `value` is [f32::NAN] or infinite. pub fn new(value: f32) -> Result { - if value.is_nan() { - return Err(Error::ReadingError(FloatIsNaN.into())); + if !value.is_finite() { + return Err(ReadingError::InvalidFloat.into()); } Ok(Float(value)) } - /// Wraps the given [f32]-`value`, that is a number, as a value over [Float]. + /// Wraps the given [f32]-`value` as a value over [Float]. /// /// # Panics - /// The given `value` is [f32::NAN]. + /// Panics if `value` is [f32::NAN] or not finite. pub fn from_number(value: f32) -> Self { - if value.is_nan() { - panic!("The provided value is not a number (NaN)!") + if !value.is_finite() { + panic!("floating point values must be finite") } Float(value) @@ -43,7 +52,7 @@ impl Float { /// Computes the absolute value. pub(crate) fn abs(self) -> Self { - Float::new(self.0.abs()).expect("Taking the absolute value cannot result in NaN") + Float::new(self.0.abs()).expect("operation returned in invalid float") } /// Returns the logarithm of the number with respect to an arbitrary base. @@ -53,33 +62,33 @@ impl Float { /// Computes the sine of a number (in radians). pub(crate) fn sin(self) -> Self { - Float::new(self.0.sin()).expect("Operation does not result in NaN") + Float::new(self.0.sin()).expect("operation returned in invalid float") } /// Computes the cosine of a number (in radians). pub(crate) fn cos(self) -> Self { - Float::new(self.0.cos()).expect("Operation does not result in NaN") + Float::new(self.0.cos()).expect("operation returned in invalid float") } /// Computes the tangent of a number (in radians). pub(crate) fn tan(self) -> Self { - Float::new(self.0.tan()).expect("Operation does not result in NaN") + Float::new(self.0.tan()).expect("operation returned in invalid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn round(self) -> Self { - Float::new(self.0.round()).expect("Operation does not result in NaN") + Float::new(self.0.round()).expect("operation returned in invalid float") } /// Returns the smallest integer greater than or equal to `self`. pub(crate) fn ceil(self) -> Self { - Float::new(self.0.ceil()).expect("Operation does not result in NaN") + Float::new(self.0.ceil()).expect("operation returned in invalid float") } /// Returns the largest integer less than or equal to `self`. pub(crate) fn floor(self) -> Self { - Float::new(self.0.floor()).expect("Operation does not result in NaN") + Float::new(self.0.floor()).expect("operation returned in invalid float") } } @@ -95,7 +104,7 @@ impl Ord for Float { fn cmp(&self, other: &Self) -> Ordering { self.0 .partial_cmp(&other.0) - .expect("Comparison can only fail on NaN values which have been forbidden in this type") + .expect("comparison can only fail on NaN values which have been forbidden in this type") } } @@ -293,7 +302,7 @@ impl Bounded for Float { impl Arbitrary for Float { fn arbitrary(g: &mut Gen) -> Self { let mut value = f32::arbitrary(g); - while value.is_nan() { + while !value.is_finite() { value = f32::arbitrary(g); } diff --git a/nemo-physical/src/datatypes/float_is_nan.rs b/nemo-physical/src/datatypes/float_is_nan.rs deleted file mode 100644 index c45fc7e63..000000000 --- a/nemo-physical/src/datatypes/float_is_nan.rs +++ /dev/null @@ -1,18 +0,0 @@ -use std::fmt; -use std::fmt::Formatter; - -/// Error for cases when [f32::NAN`] or [`f64::NAN], values that are not allowed, -/// are being cast to [super::Float`] or [`super::Double], respectively. -#[derive(Clone, Copy, Debug, Ord, PartialOrd, Eq, PartialEq)] -pub struct FloatIsNaN; - -impl fmt::Display for FloatIsNaN { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!( - f, - "The floating point types used in this library do not support NaN!" - ) - } -} - -impl std::error::Error for FloatIsNaN {} diff --git a/nemo-physical/src/error.rs b/nemo-physical/src/error.rs index 7f0547baf..c1b8494dc 100644 --- a/nemo-physical/src/error.rs +++ b/nemo-physical/src/error.rs @@ -4,7 +4,7 @@ use std::{convert::Infallible, fmt::Display}; use thiserror::Error; -use crate::{datatypes::FloatIsNaN, resource::Resource}; +use crate::resource::Resource; /// Trait that can be used by external libraries extending Nemo to communicate a error during reading pub trait ExternalReadingError: Display + std::fmt::Debug {} @@ -17,8 +17,8 @@ pub enum ReadingError { #[error(transparent)] ExternalError(#[from] Box), /// Error from trying to use a floating point number that is NaN - #[error(transparent)] - FloatIsNaN(#[from] FloatIsNaN), + #[error("floating point values must be finite and not NaN")] + InvalidFloat, /// Error occurred during parsing of Int values #[error(transparent)] ParseInt(#[from] std::num::ParseIntError), From 071a55560f5e4394d76a21ec02646b57ba3f2b7f Mon Sep 17 00:00:00 2001 From: Alex Ivliev Date: Fri, 10 Jan 2025 15:26:33 +0100 Subject: [PATCH 2/3] add testcase for infinity and NaN --- .../regression/builtin/float-nan-inf/run.rls | 11 +++++++++++ .../regression/builtin/float-nan-inf/run/result.csv | 0 2 files changed, 11 insertions(+) create mode 100644 resources/testcases/regression/builtin/float-nan-inf/run.rls create mode 100644 resources/testcases/regression/builtin/float-nan-inf/run/result.csv diff --git a/resources/testcases/regression/builtin/float-nan-inf/run.rls b/resources/testcases/regression/builtin/float-nan-inf/run.rls new file mode 100644 index 000000000..92b8f4458 --- /dev/null +++ b/resources/testcases/regression/builtin/float-nan-inf/run.rls @@ -0,0 +1,11 @@ +%! This test case is based on https://github.com/knowsys/nemo/issues/582. +%! A crash was caused by not correctly checking for NaN and infinity when +%! doing computations with floating point numbers. + +big(1.7976931348623157E308) . +small(1.0E-32) . +small(0.0) . + +result(?a / ?b) :- big(?a), small(?b) . + +@export result :- csv {} . \ No newline at end of file diff --git a/resources/testcases/regression/builtin/float-nan-inf/run/result.csv b/resources/testcases/regression/builtin/float-nan-inf/run/result.csv new file mode 100644 index 000000000..e69de29bb From ff94d26bf110bd8654112c19a137d90c41229a7d Mon Sep 17 00:00:00 2001 From: Alex Ivliev Date: Mon, 13 Jan 2025 09:51:34 +0100 Subject: [PATCH 3/3] suggestions and clippy --- nemo-physical/src/datatypes/double.rs | 20 +++++++++---------- nemo-physical/src/datatypes/float.rs | 28 +++++++++++++-------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/nemo-physical/src/datatypes/double.rs b/nemo-physical/src/datatypes/double.rs index a422254c6..894c5381d 100644 --- a/nemo-physical/src/datatypes/double.rs +++ b/nemo-physical/src/datatypes/double.rs @@ -22,7 +22,7 @@ use super::{run_length_encodable::FloatingStep, FloorToUsize, RunLengthEncodable #[cfg(test)] use quickcheck::{Arbitrary, Gen}; -/// Wrapper for [f64`] that does not allow [`f64::NAN] values. +/// Wrapper for [f64] that excludes [f64::NAN] and infinite values #[derive(Copy, Clone, Debug, PartialEq, Default)] pub struct Double(f64); @@ -33,7 +33,7 @@ impl Double { /// Returns an error if `value` is [f32::NAN] or infinite. pub fn new(value: f64) -> Result { if !value.is_finite() { - return Err(ReadingError::InvalidFloat.into()); + return Err(ReadingError::InvalidFloat); } Ok(Self(value)) @@ -44,7 +44,7 @@ impl Double { /// # Panics /// Panics if `value` is [f64::NAN] or not finite. pub fn from_number(value: f64) -> Self { - if value.is_nan() { + if !value.is_finite() { panic!("floating point values must be finite") } @@ -53,7 +53,7 @@ impl Double { /// Computes the absolute value. pub(crate) fn abs(self) -> Self { - Double::new(self.0.abs()).expect("operation returned in invalid float") + Double::new(self.0.abs()).expect("operation returns valid float") } /// Returns the logarithm of the number with respect to an arbitrary base. @@ -63,34 +63,34 @@ impl Double { /// Computes the sine of a number (in radians). pub(crate) fn sin(self) -> Self { - Double::new(self.0.sin()).expect("operation returned in invalid float") + Double::new(self.0.sin()).expect("operation returns valid float") } /// Computes the cosine of a number (in radians). pub(crate) fn cos(self) -> Self { - Double::new(self.0.cos()).expect("operation returned in invalid float") + Double::new(self.0.cos()).expect("operation returns valid float") } /// Computes the tangent of a number (in radians). pub(crate) fn tan(self) -> Self { - Double::new(self.0.tan()).expect("operation returned in invalid float") + Double::new(self.0.tan()).expect("operation returns valid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn round(self) -> Self { - Double::new(self.0.round()).expect("operation returned in invalid float") + Double::new(self.0.round()).expect("operation returns valid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn ceil(self) -> Self { - Double::new(self.0.ceil()).expect("operation returned in invalid float") + Double::new(self.0.ceil()).expect("operation returns valid float") } /// Returns the largest integer less than or equal to `self`. pub(crate) fn floor(self) -> Self { - Double::new(self.0.floor()).expect("operation returned in invalid float") + Double::new(self.0.floor()).expect("operation returns valid float") } } diff --git a/nemo-physical/src/datatypes/float.rs b/nemo-physical/src/datatypes/float.rs index 1e1a48c22..5e8892a98 100644 --- a/nemo-physical/src/datatypes/float.rs +++ b/nemo-physical/src/datatypes/float.rs @@ -21,7 +21,7 @@ use crate::{ #[cfg(test)] use quickcheck::{Arbitrary, Gen}; -/// Wrapper for [f32] that does not allow [f32::NAN] values. +/// Wrapper for [f32] that excludes [f32::NAN] and infinite values #[derive(Copy, Clone, Debug, PartialEq, Default)] pub struct Float(f32); @@ -30,9 +30,9 @@ impl Float { /// /// # Errors /// Returns an error if `value` is [f32::NAN] or infinite. - pub fn new(value: f32) -> Result { + pub fn new(value: f32) -> Result { if !value.is_finite() { - return Err(ReadingError::InvalidFloat.into()); + return Err(ReadingError::InvalidFloat); } Ok(Float(value)) @@ -52,7 +52,7 @@ impl Float { /// Computes the absolute value. pub(crate) fn abs(self) -> Self { - Float::new(self.0.abs()).expect("operation returned in invalid float") + Float::new(self.0.abs()).expect("operation returns valid float") } /// Returns the logarithm of the number with respect to an arbitrary base. @@ -62,33 +62,33 @@ impl Float { /// Computes the sine of a number (in radians). pub(crate) fn sin(self) -> Self { - Float::new(self.0.sin()).expect("operation returned in invalid float") + Float::new(self.0.sin()).expect("operation returns valid float") } /// Computes the cosine of a number (in radians). pub(crate) fn cos(self) -> Self { - Float::new(self.0.cos()).expect("operation returned in invalid float") + Float::new(self.0.cos()).expect("operation returns valid float") } /// Computes the tangent of a number (in radians). pub(crate) fn tan(self) -> Self { - Float::new(self.0.tan()).expect("operation returned in invalid float") + Float::new(self.0.tan()).expect("operation returns valid float") } /// Returns the nearest integer to `self`. /// If a value is half-way between two integers, round away from 0.0. pub(crate) fn round(self) -> Self { - Float::new(self.0.round()).expect("operation returned in invalid float") + Float::new(self.0.round()).expect("operation returns valid float") } /// Returns the smallest integer greater than or equal to `self`. pub(crate) fn ceil(self) -> Self { - Float::new(self.0.ceil()).expect("operation returned in invalid float") + Float::new(self.0.ceil()).expect("operation returns valid float") } /// Returns the largest integer less than or equal to `self`. pub(crate) fn floor(self) -> Self { - Float::new(self.0.floor()).expect("operation returned in invalid float") + Float::new(self.0.floor()).expect("operation returns valid float") } } @@ -179,7 +179,7 @@ impl fmt::Display for Float { } impl TryFrom for Float { - type Error = Error; + type Error = ReadingError; fn try_from(value: f32) -> Result { Self::new(value) @@ -196,9 +196,9 @@ impl TryFrom for Float { type Error = Error; fn try_from(value: usize) -> Result { - f32::from_usize(value) - .ok_or(Error::UsizeToFloatingPointValue(value)) - .and_then(Float::new) + let res = f32::from_usize(value).ok_or(Error::UsizeToFloatingPointValue(value))?; + + Ok(Float::new(res)?) } }