Skip to content

Commit

Permalink
Fix comparison operators for double values in DS (#9735)
Browse files Browse the repository at this point in the history 
* fix for equality for double values in DS

* adjust tolerance value, add tests

* preserve access to prevent API break

* fix < and > operators

* use Double.CompareTo, more refactoring

* update tests

* address review comments

* add tests, restore deleted methods as obsolete
  • Loading branch information
@aparajit-pratap
aparajit-pratap authored Jul 23, 2019
1 parent f8cfc0a commit 83042e2
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 33 deletions.
6 changes: 3 additions & 3 deletions src/Engine/ProtoCore/DSASM/Executive.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3880,7 +3880,7 @@ private void LT_Handler(Instruction instruction)
{
double value1 = opdata2.IsDouble ? opdata2.DoubleValue: opdata2.IntegerValue;
double value2 = opdata1.IsDouble ? opdata1.DoubleValue: opdata1.IntegerValue;
opdata2 = StackValue.BuildBoolean(MathUtils.IsLessThan(value1, value2));
opdata2 = StackValue.BuildBoolean(value1 < value2);
}
else
{
Expand All @@ -3902,7 +3902,7 @@ private void GE_Handler(Instruction instruction)
{
double lhs = opdata2.IsDouble ? opdata2.DoubleValue : opdata2.IntegerValue;
double rhs = opdata1.IsDouble ? opdata1.DoubleValue : opdata1.IntegerValue;
opdata2 = StackValue.BuildBoolean(MathUtils.IsGreaterThanOrEquals(lhs, rhs));
opdata2 = StackValue.BuildBoolean(lhs >= rhs);
}
else
{
Expand All @@ -3929,7 +3929,7 @@ private void LE_Handler(Instruction instruction)
{
double lhs = opdata2.IsDouble ? opdata2.DoubleValue: opdata2.IntegerValue;
double rhs = opdata1.IsDouble ? opdata1.DoubleValue: opdata1.IntegerValue;
opdata2 = StackValue.BuildBoolean(MathUtils.IsLessThanOrEquals(lhs, rhs));
opdata2 = StackValue.BuildBoolean(lhs <= rhs);
}
else
{
Expand Down
2 changes: 0 additions & 2 deletions src/Engine/ProtoCore/DSASM/Stack.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,6 @@ public static bool CompareStackValues(StackValue sv1, StackValue sv2, RuntimeCor
var value1 = sv1.DoubleValue;
var value2 = sv2.DoubleValue;

if(Double.IsInfinity(value1) && Double.IsInfinity(value2))
return true;
return MathUtils.Equals(value1, value2);
case AddressType.Boolean:
return sv1.BooleanValue == sv2.BooleanValue;
Expand Down
24 changes: 3 additions & 21 deletions src/Engine/ProtoCore/Lang/BuiltInFunctionEndPoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2575,30 +2575,15 @@ internal static StackValue Evaluate(StackValue function, StackValue parameters,

class StackValueComparerForDouble : IComparer<StackValue>
{
private bool mbAscending = true;
private readonly bool mbAscending = true;
public StackValueComparerForDouble(bool ascending)
{
mbAscending = ascending;
}

bool Equals(StackValue sv1, StackValue sv2)
{
bool sv1null = !sv1.IsNumeric;
bool sv2null = !sv2.IsNumeric;
if ( sv1null && sv2null)
return true;
if (sv1null || sv2null)
return false;

var v1 = sv1.IsDouble ? sv1.DoubleValue: sv1.IntegerValue;
var v2 = sv2.IsDouble ? sv2.DoubleValue: sv2.IntegerValue;

return MathUtils.Equals(v1, v2);
}

public int Compare(StackValue sv1, StackValue sv2)
{
if (Equals(sv1, sv2))
if (!sv1.IsNumeric && !sv2.IsNumeric)
return 0;

if (!sv1.IsNumeric)
Expand All @@ -2613,10 +2598,7 @@ public int Compare(StackValue sv1, StackValue sv2)
double x = mbAscending ? value1 : value2;
double y = mbAscending ? value2 : value1;

if (x > y)
return 1;
else
return -1;
return x.CompareTo(y);
}
}
}
16 changes: 13 additions & 3 deletions src/Engine/ProtoCore/Utils/MathUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,40 @@ namespace ProtoCore.Utils
{
public static class MathUtils
{
public static double Tolerance = 1e-5;
public static double Tolerance = 1e-5;

[Obsolete("This method is no longer used. Remove in Dynamo 3.0")]
public static bool IsLessThan(double lhs, double rhs)
{
return (lhs < rhs) && !Equals(lhs, rhs);
return (lhs<rhs) && !Equals(lhs, rhs);
}

[Obsolete("This method is no longer used. Remove in Dynamo 3.0")]
public static bool IsGreaterThan(double lhs, double rhs)
{
return (lhs > rhs) && !Equals(lhs, rhs);
}

[Obsolete("This method is no longer used. Remove in Dynamo 3.0")]
public static bool IsGreaterThanOrEquals(double lhs, double rhs)
{
return (lhs > rhs) || Equals(lhs, rhs);
return (lhs > rhs) || Equals(lhs, rhs);
}

[Obsolete("This method is no longer used. Remove in Dynamo 3.0")]
public static bool IsLessThanOrEquals(double lhs, double rhs)
{
return (lhs < rhs) || Equals(lhs, rhs);
}

public static bool Equals(double lhs, double rhs)
{
if (double.IsPositiveInfinity(lhs) && double.IsPositiveInfinity(rhs))
return true;

if (double.IsNegativeInfinity(lhs) && double.IsNegativeInfinity(rhs))
return true;

return Math.Abs(lhs - rhs) < Tolerance;
}
}
Expand Down
18 changes: 18 additions & 0 deletions test/Engine/ProtoTest/Associative/MicroFeatureTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3670,5 +3670,23 @@ public void TestReturnStatement16()
thisTest.RunScriptSource(code);
thisTest.VerifyBuildWarningCount(0);
}

[Test]
public void TestDoubleEquality()
{
string code = @"
a = 1/3 == 0.33333;
b = 1/3 == 0.3333;
c = 3.14159265358979;
d = c == c + 0.000001;
e = c == c + 0.00001;
";
thisTest.RunScriptSource(code);
thisTest.VerifyBuildWarningCount(0);
thisTest.Verify("a", true);
thisTest.Verify("b", false);
thisTest.Verify("d", true);
thisTest.Verify("e", false);
}
}
}
49 changes: 47 additions & 2 deletions test/Engine/ProtoTest/Imperative/MicroFeatureTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ public void Factorial01()
}

[Test]
public void ToleranceTest()
public void ToleranceTestLessThan()
{
String code =
@"a=[Imperative]
Expand All @@ -904,7 +904,52 @@ public void ToleranceTest()
}
";
thisTest.RunScriptSource(code);
thisTest.Verify("a",0.3);
thisTest.Verify("a",0.0);
}

[Test]
public void ToleranceTestGreaterThan()
{
String code =
@"a=[Imperative]
{
a = 0.3; b = 0.1;
if (0.2 > a-b) { a = 0; }
return a;
}
";
thisTest.RunScriptSource(code);
thisTest.Verify("a", 0.0);
}

[Test]
public void ToleranceTestLessThanEquals()
{
String code =
@"a=[Imperative]
{
a = 0.123456; b = 0.123455;
if (a <= b) { a = 0; }
return a;
}
";
thisTest.RunScriptSource(code);
thisTest.Verify("a", 0.12345);
}

[Test]
public void ToleranceTestGreaterThanEquals()
{
String code =
@"a=[Imperative]
{
a = 0.123456; b = 0.123455;
if (b >= a) { a = 0; }
return a;
}
";
thisTest.RunScriptSource(code);
thisTest.Verify("a", 0.123456);
}

[Test]
Expand Down
9 changes: 7 additions & 2 deletions test/Engine/ProtoTest/TD/DocTests/UserManualTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,18 @@ public void UM33_NumberTypes()
@"
// create two seemingly different numbers
a = 1.0;
b = 0.9999999;
b = 0.99999;
// test if the two numbers are equal
s = Print(a == b ? ""Are equal"" : ""Are not equal"");
x = a == b ? 1 : 0;
c = 1.0;
d = 0.9999;
// test if the two numbers are equal
y = c == d ? 1 : 0;
";
thisTest.RunScriptSource(code);
thisTest.Verify("x", 1);
thisTest.Verify("y", 0);
}

[Test]
Expand Down

0 comments on commit 83042e2

Please sign in to comment.