Skip to content

Commit 5d9a140

Browse files
ducky64ducky64
authored
Improve empty range handling (#460)
Addition with empty ranges now results in the empty range, and sum on empty range is defined consistently. Sum on empty array now defined as 0, consistent with the initial accumulator value interpretation. Does not change any examples, empty is only used for analog signal values (single value only) and input thresholds (hulled). Improves test coverage of empty cases.
1 parent 6fe8909 commit 5d9a140

4 files changed

Lines changed: 46 additions & 15 deletions

File tree

‎compiler/src/main/scala/edg/ExprBuilder.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ object ExprBuilder {
3434
def Literal(valueMin: Double, valueMax: Double): expr.ValueExpr = // convenience method
3535
Literal(valueMin.toFloat, valueMax.toFloat)
3636

37+
def LiteralRangeEmpty(): expr.ValueExpr =
38+
Literal(Float.NaN, Float.NaN)
39+
3740
def BinOp(op: expr.BinaryExpr.Op, lhs: expr.ValueExpr, rhs: expr.ValueExpr): expr.ValueExpr = expr.ValueExpr(
3841
expr = expr.ValueExpr.Expr.Binary(expr.BinaryExpr(op = op, lhs = Some(lhs), rhs = Some(rhs)))
3942
)

‎compiler/src/main/scala/edg/compiler/ExprEvaluate.scala‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ object ExprEvaluate {
2121
case (RangeValue(lhsMin, lhsMax), RangeValue(rhsMin, rhsMax)) =>
2222
val all = Seq(lhsMin + rhsMin, lhsMin + rhsMax, lhsMax + rhsMin, lhsMax + rhsMax)
2323
RangeValue(all.min, all.max)
24-
case (RangeEmpty, RangeEmpty) => RangeEmpty
25-
case (lhs: RangeValue, RangeEmpty) => lhs
26-
case (RangeEmpty, rhs: RangeValue) => rhs
24+
case (_: RangeType, RangeEmpty) => RangeEmpty
25+
case (RangeEmpty, _: RangeType) => RangeEmpty
2726
case (RangeValue(lhsMin, lhsMax), FloatPromotable(rhs)) =>
2827
RangeValue(lhsMin + rhs, lhsMax + rhs)
2928
case (FloatPromotable(lhs), RangeValue(rhsMin, rhsMax)) =>
@@ -39,9 +38,8 @@ object ExprEvaluate {
3938
case (RangeValue(lhsMin, lhsMax), RangeValue(rhsMin, rhsMax)) =>
4039
val all = Seq(lhsMin * rhsMin, lhsMin * rhsMax, lhsMax * rhsMin, lhsMax * rhsMax)
4140
RangeValue(all.min, all.max)
42-
case (RangeEmpty, RangeEmpty) => RangeEmpty
43-
case (lhs: RangeValue, RangeEmpty) => RangeEmpty
44-
case (RangeEmpty, rhs: RangeValue) => RangeEmpty
41+
case (_: RangeType, RangeEmpty) => RangeEmpty
42+
case (RangeEmpty, _: RangeType) => RangeEmpty
4543
case (RangeValue(lhsMin, lhsMax), FloatPromotable(rhs)) if rhs >= 0 =>
4644
RangeValue(lhsMin * rhs, lhsMax * rhs)
4745
case (RangeValue(lhsMin, lhsMax), FloatPromotable(rhs)) if rhs < 0 =>
@@ -170,8 +168,8 @@ object ExprEvaluate {
170168
}
171169

172170
case Op.INTERSECTION => (lhs, rhs) match {
173-
case (RangeEmpty, _) => RangeEmpty // anything intersecting with empty is empty
174-
case (_, RangeEmpty) => RangeEmpty
171+
case (RangeEmpty, _: RangeType) => RangeEmpty // anything intersecting with empty is empty
172+
case (_: RangeType, RangeEmpty) => RangeEmpty
175173
case (RangeValue(lhsMin, lhsMax), RangeValue(rhsMin, rhsMax)) =>
176174
val (minMax, maxMin) = (math.min(lhsMax, rhsMax), math.max(lhsMin, rhsMin))
177175
if (maxMin <= minMax) {
@@ -305,7 +303,10 @@ object ExprEvaluate {
305303
case (Op.SUM, ArrayValue.ExtractBoolean(vals)) => IntValue(vals.count(_ == true))
306304
case (Op.SUM, ArrayValue.UnpackRange(extracted)) => extracted match {
307305
case ArrayValue.UnpackRange.FullRange(valMins, valMaxs) => RangeValue(valMins.sum, valMaxs.sum)
308-
case _ => ErrorValue("unpack_range(empty) is undefined")
306+
case ArrayValue.UnpackRange.RangeWithEmpty(_, _) => RangeEmpty
307+
case ArrayValue.UnpackRange.EmptyRange() => RangeEmpty
308+
case ArrayValue.UnpackRange.EmptyArray() =>
309+
RangeValue(0, 0) // unreachable in practice, superseded by float 0 case
309310
}
310311

311312
case (Op.ALL_TRUE, ArrayValue.Empty(_)) => BooleanValue(true)
@@ -340,8 +341,9 @@ object ExprEvaluate {
340341
} else { // does not intersect, null set
341342
ErrorValue(f"intersection($extracted) produces empty set")
342343
}
344+
case ArrayValue.UnpackRange.RangeWithEmpty(_, _) => RangeEmpty
345+
case ArrayValue.UnpackRange.EmptyRange() => RangeEmpty
343346
// The implicit initial value of intersect is the full range
344-
// TODO are these good semantics?
345347
case ArrayValue.UnpackRange.EmptyArray() => RangeValue(Float.NegativeInfinity, Float.PositiveInfinity)
346348
case _ => ErrorValue(f"intersection($vals) is undefined")
347349
}
@@ -350,8 +352,8 @@ object ExprEvaluate {
350352
case (Op.HULL, ArrayValue.UnpackRange(extracted)) => extracted match {
351353
case ArrayValue.UnpackRange.FullRange(valMins, valMaxs) => RangeValue(valMins.min, valMaxs.max)
352354
case ArrayValue.UnpackRange.RangeWithEmpty(valMins, valMaxs) => RangeValue(valMins.min, valMaxs.max)
353-
case ArrayValue.UnpackRange.EmptyArray() => RangeEmpty // TODO: should this be an error?
354355
case ArrayValue.UnpackRange.EmptyRange() => RangeEmpty
356+
case ArrayValue.UnpackRange.EmptyArray() => RangeEmpty // TODO: should this be an error?
355357
}
356358
case (Op.HULL, ArrayValue.ExtractFloat(vals)) => RangeValue(vals.min, vals.max)
357359

‎compiler/src/test/scala/edg/compiler/ExprEvaluateTest.scala‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ class ExprEvaluateTest extends AnyFlatSpec {
1212
val constProp = new ConstProp()
1313
val evalTest = new ExprEvaluate(constProp, DesignPath())
1414

15-
// TODO: add array tests once there is an array literal
16-
1715
it should "handle literals" in {
1816
evalTest.map(ValueExpr.Literal(Literal.Floating(2.0))) should equal(FloatValue(2.0))
1917
evalTest.map(ValueExpr.Literal(Literal.Integer(42))) should equal(IntValue(42))
@@ -97,6 +95,17 @@ class ExprEvaluateTest extends AnyFlatSpec {
9795

9896
it should "handle binary range ops" in {
9997
import edgir.expr.expr.BinaryExpr.Op
98+
99+
evalTest.map(
100+
ValueExpr.BinOp(Op.ADD, ValueExpr.Literal(4.0, 6.0), ValueExpr.Literal(5.0, 7.0))
101+
) should equal(RangeValue(9.0, 13.0))
102+
evalTest.map(
103+
ValueExpr.BinOp(Op.ADD, ValueExpr.Literal(5.0, 7.0), ValueExpr.LiteralRangeEmpty())
104+
) should equal(RangeEmpty)
105+
evalTest.map(
106+
ValueExpr.BinOp(Op.ADD, ValueExpr.LiteralRangeEmpty(), ValueExpr.LiteralRangeEmpty())
107+
) should equal(RangeEmpty)
108+
100109
evalTest.map(
101110
ValueExpr.BinOp(Op.INTERSECTION, ValueExpr.Literal(4.0, 6.0), ValueExpr.Literal(5.0, 7.0))
102111
) should equal(RangeValue(5.0, 6.0))
@@ -109,7 +118,12 @@ class ExprEvaluateTest extends AnyFlatSpec {
109118
evalTest.map(
110119
ValueExpr.BinOp(Op.INTERSECTION, ValueExpr.Literal(5.0, 7.0), ValueExpr.Literal(8.0, 10.0))
111120
) should equal(RangeEmpty)
112-
// TODO test with empty ranges?
121+
evalTest.map(
122+
ValueExpr.BinOp(Op.INTERSECTION, ValueExpr.Literal(5.0, 7.0), ValueExpr.LiteralRangeEmpty())
123+
) should equal(RangeEmpty)
124+
evalTest.map(
125+
ValueExpr.BinOp(Op.INTERSECTION, ValueExpr.LiteralRangeEmpty(), ValueExpr.LiteralRangeEmpty())
126+
) should equal(RangeEmpty)
113127

114128
evalTest.map(ValueExpr.BinOp(Op.HULL, ValueExpr.Literal(4.0, 6.0), ValueExpr.Literal(5.0, 7.0))) should equal(
115129
RangeValue(4.0, 7.0)
@@ -123,7 +137,12 @@ class ExprEvaluateTest extends AnyFlatSpec {
123137
evalTest.map(ValueExpr.BinOp(Op.HULL, ValueExpr.Literal(5.0, 7.0), ValueExpr.Literal(8.0, 10.0))) should equal(
124138
RangeValue(5.0, 10.0)
125139
)
126-
// TODO test with empty ranges?
140+
evalTest.map(
141+
ValueExpr.BinOp(Op.HULL, ValueExpr.Literal(5.0, 7.0), ValueExpr.LiteralRangeEmpty())
142+
) should equal(RangeValue(5.0, 7.0))
143+
evalTest.map(
144+
ValueExpr.BinOp(Op.HULL, ValueExpr.LiteralRangeEmpty(), ValueExpr.LiteralRangeEmpty())
145+
) should equal(RangeEmpty)
127146

128147
evalTest.map(ValueExpr.BinOp(Op.WITHIN, ValueExpr.Literal(6.0), ValueExpr.Literal(5.0, 7.0))) should equal(
129148
BooleanValue(true)
@@ -150,6 +169,13 @@ class ExprEvaluateTest extends AnyFlatSpec {
150169
evalTest.map(ValueExpr.BinOp(Op.WITHIN, ValueExpr.Literal(5.5, 7.5), ValueExpr.Literal(5.0, 7.0))) should equal(
151170
BooleanValue(false)
152171
)
172+
173+
evalTest.map(ValueExpr.BinOp(Op.WITHIN, ValueExpr.Literal(5.5, 7.5), ValueExpr.LiteralRangeEmpty())) should equal(
174+
BooleanValue(false)
175+
)
176+
evalTest.map(ValueExpr.BinOp(Op.WITHIN, ValueExpr.LiteralRangeEmpty(), ValueExpr.Literal(5.0, 7.0))) should equal(
177+
BooleanValue(true)
178+
)
153179
}
154180

155181
it should "handle arithmetic promotions" in {

‎edg/core/resources/edg-compiler-precompiled.jar‎

53 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)