Perform unit conversion in equality, comparison and arithmetic operators for Quantity (and Ratio)

Src/java/cql-to-elm/src/commonMain/kotlin/org/cqframework/cql/cql2elm/ucum/UcumService.kt

@@ -22,6 +22,16 @@ interface UcumService {
      * @return null if valid, error message if invalid
      */
     fun validate(unit: String): String?
+
+    fun multiply(
+        left: Pair<BigDecimal, String>,
+        right: Pair<BigDecimal, String>,
+    ): Pair<BigDecimal, String>
+
+    fun divideBy(
+        left: Pair<BigDecimal, String>,
+        right: Pair<BigDecimal, String>,
+    ): Pair<BigDecimal, String>
 }
 
 expect val defaultLazyUcumService: Lazy<UcumService>
@@ -40,6 +50,8 @@ expect val defaultLazyUcumService: Lazy<UcumService>
 fun createUcumService(
     convertUnit: (value: String, sourceUnit: String, destUnit: String) -> String,
     validateUnit: (unit: String) -> String?,
+    multiply: (Pair<BigDecimal, String>, Pair<BigDecimal, String>) -> Pair<BigDecimal, String>,
+    divideBy: (Pair<BigDecimal, String>, Pair<BigDecimal, String>) -> Pair<BigDecimal, String>,
 ): Lazy<UcumService> {
     return lazy {
         object : UcumService {
@@ -55,6 +67,20 @@ fun createUcumService(
             override fun validate(unit: String): String? {
                 return validateUnit(unit)
             }
+
+            override fun multiply(
+                left: Pair<BigDecimal, String>,
+                right: Pair<BigDecimal, String>,
+            ): Pair<BigDecimal, String> {
+                return multiply(left, right)
+            }
+
+            override fun divideBy(
+                left: Pair<BigDecimal, String>,
+                right: Pair<BigDecimal, String>,
+            ): Pair<BigDecimal, String> {
+                return divideBy(left, right)
+            }
         }
     }
 }

Src/java/cql-to-elm/src/wasmJsMain/kotlin/org/cqframework/cql/cql2elm/ucum/UcumServiceWasmJs.kt

@@ -2,6 +2,8 @@
 
 package org.cqframework.cql.cql2elm.ucum
 
+import org.cqframework.cql.shared.BigDecimal
+
 actual val defaultLazyUcumService =
     lazy<UcumService> { error("No default UCUM service available.") }
 
@@ -10,6 +12,38 @@ actual val defaultLazyUcumService =
 fun createUcumServiceReference(
     convertUnit: (value: String, sourceUnit: String, destUnit: String) -> String,
     validateUnit: (unit: String) -> String?,
+    multiply:
+        (leftValue: String, leftUnit: String, rightValue: String, rightUnit: String) -> String,
+    divideBy: (leftValue: String, leftUnit: String, rightValue: String, rightUnit: String) -> String,
 ): JsReference<Lazy<UcumService>> {
-    return createUcumService(convertUnit, validateUnit).toJsReference()
+    return createUcumService(
+            convertUnit,
+            validateUnit,
+            { left, right ->
+                // The multiply function on the JS side has to encode the result as a string
+                // "<value>:<unit>" since we can
+                // pass and return only primitive values.
+                val result =
+                    multiply(
+                        left.first.toString(),
+                        left.second,
+                        right.first.toString(),
+                        right.second,
+                    )
+                val valueAndUnit = result.split(":", limit = 2)
+                Pair(BigDecimal(valueAndUnit[0]), valueAndUnit[1])
+            },
+            { left, right ->
+                val result =
+                    multiply(
+                        left.first.toString(),
+                        left.second,
+                        right.first.toString(),
+                        right.second,
+                    )
+                val valueAndUnit = result.split(":", limit = 2)
+                Pair(BigDecimal(valueAndUnit[0]), valueAndUnit[1])
+            },
+        )
+        .toJsReference()
 }

Src/java/engine-fhir/src/main/kotlin/org/opencds/cqf/cql/engine/fhir/retrieve/Dstu3FhirQueryGenerator.kt

@@ -147,10 +147,14 @@ class Dstu3FhirQueryGenerator(
                                     .withValue(dateFilterAsDuration.getValue())
                                     .withUnit(dateFilterAsDuration.getUnit())
 
+                            // Passing null as the state argument to the subtract method is fine
+                            // here since that method only uses the state when it has to convert
+                            // Quantities with different units which cannot happen here.
                             val diff =
                                 (SubtractEvaluator.subtract(
                                     evaluationDateTime,
                                     dateFilterDurationAsCQLQuantity,
+                                    null,
                                 ) as DateTime?)
 
                             dateRange = Interval(diff, true, evaluationDateTime, true)

Src/java/engine-fhir/src/main/kotlin/org/opencds/cqf/cql/engine/fhir/retrieve/R4FhirQueryGenerator.kt

@@ -125,10 +125,14 @@ class R4FhirQueryGenerator(
                                     .withValue(dateFilterAsDuration.getValue())
                                     .withUnit(dateFilterAsDuration.getUnit())
 
+                            // Passing null as the state argument to the subtract method is fine
+                            // here since that method only uses the state when it has to convert
+                            // Quantities with different units which cannot happen here.
                             val diff =
                                 (SubtractEvaluator.subtract(
                                     evaluationDateTime,
                                     dateFilterDurationAsCQLQuantity,
+                                    null,
                                 ) as DateTime?)
 
                             dateRange = Interval(diff, true, evaluationDateTime, true)

Src/java/engine-fhir/src/test/kotlin/org/hl7/fhirpath/CQLOperationsR4Test.kt

@@ -137,24 +137,11 @@ class CQLOperationsR4Test : TestFhirPath() {
         }
 
         var SKIP: MutableSet<String> =
-            Sets.newHashSet<String>(
+            Sets.newHashSet(
                 "cql/CqlAggregateTest/AggregateTests/RolledOutIntervals",
-                "cql/CqlArithmeticFunctionsTest/Divide/Divide1Q1Q",
                 "cql/CqlArithmeticFunctionsTest/Ln/Ln1000D",
                 "cql/CqlArithmeticFunctionsTest/Ln/Ln1000",
                 "cql/CqlArithmeticFunctionsTest/MinValue/LongMinValue",
-                "cql/CqlArithmeticFunctionsTest/Multiply/Multiply1CMBy2CM",
-                "cql/CqlComparisonOperatorsTest/Equal/QuantityEqCM1M01",
-                "cql/CqlComparisonOperatorsTest/Equivalent/EquivEqCM1M01",
-                "cql/CqlComparisonOperatorsTest/Greater/GreaterM1CM1",
-                "cql/CqlComparisonOperatorsTest/Greater/GreaterM1CM10",
-                "cql/CqlComparisonOperatorsTest/Greater Or Equal/GreaterOrEqualM1CM1",
-                "cql/CqlComparisonOperatorsTest/Greater Or Equal/GreaterOrEqualM1CM10",
-                "cql/CqlComparisonOperatorsTest/Less/LessM1CM1",
-                "cql/CqlComparisonOperatorsTest/Less/LessM1CM10",
-                "cql/CqlComparisonOperatorsTest/Less Or Equal/LessOrEqualM1CM1",
-                "cql/CqlComparisonOperatorsTest/Less Or Equal/LessOrEqualM1CM10",
-                "cql/CqlComparisonOperatorsTest/Not Equal/QuantityNotEqCM1M01",
                 "cql/CqlDateTimeOperatorsTest/Duration/DateTimeDurationBetweenYear",
                 "cql/CqlDateTimeOperatorsTest/Uncertainty tests/DateTimeDurationBetweenUncertainAdd",
                 "cql/CqlDateTimeOperatorsTest/Uncertainty tests/DateTimeDurationBetweenUncertainInterval",
@@ -257,24 +244,16 @@ class CQLOperationsR4Test : TestFhirPath() {
                 "r4/tests-fhir-r4/testNEquality/testNEquality15",
                 "r4/tests-fhir-r4/testNEquality/testNEquality16",
                 "r4/tests-fhir-r4/testNEquality/testNEquality17",
-                "r4/tests-fhir-r4/testNEquality/testNEquality24",
                 "r4/tests-fhir-r4/testNotEquivalent/testNotEquivalent13",
                 "r4/tests-fhir-r4/testNotEquivalent/testNotEquivalent17",
                 "r4/tests-fhir-r4/testNotEquivalent/testNotEquivalent21",
                 "r4/tests-fhir-r4/testPower/testPower3",
                 "r4/tests-fhir-r4/testPrecedence/testPrecedence3",
                 "r4/tests-fhir-r4/testPrecedence/testPrecedence4",
-                "r4/tests-fhir-r4/testQuantity/testQuantity1",
-                "r4/tests-fhir-r4/testQuantity/testQuantity2",
-                "r4/tests-fhir-r4/testQuantity/testQuantity3",
-                "r4/tests-fhir-r4/testQuantity/testQuantity4",
                 "r4/tests-fhir-r4/testQuantity/testQuantity5",
                 "r4/tests-fhir-r4/testQuantity/testQuantity6",
                 "r4/tests-fhir-r4/testQuantity/testQuantity7",
                 "r4/tests-fhir-r4/testQuantity/testQuantity8",
-                "r4/tests-fhir-r4/testQuantity/testQuantity9",
-                "r4/tests-fhir-r4/testQuantity/testQuantity10",
-                "r4/tests-fhir-r4/testQuantity/testQuantity11",
                 "r4/tests-fhir-r4/testRepeat/testRepeat1",
                 "r4/tests-fhir-r4/testRepeat/testRepeat2",
                 "r4/tests-fhir-r4/testRepeat/testRepeat3",

Src/java/engine-fhir/src/test/resources/org/hl7/fhirpath/cql/CqlArithmeticFunctionsTest.xml

@@ -52,6 +52,10 @@
 			<expression>1'g/cm3' + 1'g/cm3'</expression>
 			<output>2.0'g/cm3'</output>
 		</test>
+		<test name="Add1Q1QDifferentUnits">
+			<expression>1'kg/m3' + 1'g/cm3'</expression>
+			<output>1001.0'kg/m3'</output>
+		</test>
 		<test name="AddIAndD">
 			<expression>1 + 2.0</expression>
 			<output>3.0</output>
@@ -137,7 +141,6 @@
 			should state clearly that the expected UCUM unit would be the
 			default unit (`1`). -->
 			<!-- See https://github.com/cqframework/clinical_quality_language/issues/193 -->
-			<!-- TODO: make Translator recognize unit changes on Divide() -->
 		</test>
 		<test name="Divide10I5D">
 			<expression>10 / 5.0</expression>
@@ -155,6 +158,7 @@
 			<expression>10.0 'g' / 0</expression>
 			<output>null</output>
 		</test>
+                <!-- TODO -->
 	</group>
 	<group name="Floor">
 		<test name="FloorNull">

Src/java/engine-fhir/src/test/resources/org/hl7/fhirpath/cql/CqlComparisonOperatorsTest.xml

@@ -92,6 +92,10 @@
 			<expression>2.0'cm' = 2.00'cm'</expression>
 			<output>true</output>
 		</test>
+		<test name="QuantityEqNotComparable">
+			<expression>1'm' = 1's'</expression>
+			<output>null</output>
+		</test>
 		<test name="RatioEqual">
 			<expression>1'cm':2'cm' = 1'cm':2'cm'</expression>
 			<output>true</output>
@@ -714,14 +718,18 @@
 			<expression>1.0 ~ 2</expression>
 			<output>false</output>
 		</test>
-		<test name="EquivEqCM1CM1">
+		<test name="EquivQuantityCM1CM1">
 			<expression>1'cm' ~ 1'cm'</expression>
 			<output>true</output>
 		</test>
-		<test name="EquivEqCM1M01">
+		<test name="EquivQuantityCM1M01">
 			<expression>1'cm' ~ 0.01'm'</expression>
 			<output>true</output>
 		</test>
+		<test name="EquivQuantityNotComparable">
+			<expression>1'm' ~ 1's'</expression>
+			<output>false</output>
+		</test>
 		<test name="RatioEquivalent">
 			<expression>1'cm':2'cm' ~ 1'cm':2'cm'</expression>
 			<output>true</output>
@@ -738,6 +746,14 @@
 			<expression>1'cm':2'cm' ~ 1'cm':3'cm'</expression>
 			<output>false</output>
 		</test>
+		<test name="RatioEquivalentWithDifferentUnits">
+			<expression>1'cm':0.02'm' ~ 30'mm':6'cm'</expression>
+			<output>true</output>
+		</test>
+		<test name="RatioNotEquivalentWithDifferentUnits">
+			<expression>1'cm':0.02'm' ~ 10'mm':3'cm'</expression>
+			<output>false</output>
+		</test>
 		<test name="EquivTupleJohnJohn">
 			<expression>Tuple { Id : 1, Name : 'John' } ~ Tuple { Id : 1, Name : 'John' }</expression>
 			<output>true</output>

Src/java/engine/src/main/kotlin/org/opencds/cqf/cql/engine/elm/executing/AddEvaluator.kt

@@ -2,25 +2,31 @@ package org.opencds.cqf.cql.engine.elm.executing
 
 import java.math.BigDecimal
 import org.opencds.cqf.cql.engine.exception.InvalidOperatorArgument
+import org.opencds.cqf.cql.engine.execution.State
 import org.opencds.cqf.cql.engine.runtime.*
 
 object AddEvaluator {
     @JvmStatic
-    fun add(left: Any?, right: Any?): Any? {
+    fun add(left: Any?, right: Any?, state: State?): Any? {
         if (left == null || right == null) {
             return null
         }
 
         if (left is Int && right is Int) {
             return left + right
-        }
-
-        if (left is Long && right is Long) {
+        } else if (left is Long && right is Long) {
             return left + right
         } else if (left is BigDecimal && right is BigDecimal) {
             return Value.verifyPrecision(left.add(right), null)
         } else if (left is Quantity && right is Quantity) {
-            return Quantity().withValue((left.value)!!.add(right.value)).withUnit(left.unit)
+            return computeWithConvertedUnits(
+                left,
+                right,
+                { commonUnit, leftValue, rightValue ->
+                    Quantity().withUnit(commonUnit).withValue(leftValue.add(rightValue))
+                },
+                state!!,
+            )
         } else if (left is BaseTemporal && right is Quantity) {
             var valueToAddPrecision = Precision.fromString(right.unit!!)
             var precision = Precision.fromString(BaseTemporal.getLowestPrecision(left))
@@ -69,12 +75,10 @@ object AddEvaluator {
                 )
             }
         } else if (left is Interval && right is Interval) {
-            val leftInterval = left
-            val rightInterval = right
             return Interval(
-                add(leftInterval.start, rightInterval.start),
+                add(left.start, right.start, state),
                 true,
-                add(leftInterval.end, rightInterval.end),
+                add(left.end, right.end, state),
                 true,
             )
         } else if (left is String && right is String) {

Src/java/engine/src/main/kotlin/org/opencds/cqf/cql/engine/elm/executing/AvgEvaluator.kt

@@ -22,11 +22,10 @@ object AvgEvaluator {
         }
 
         if (source is Iterable<*>) {
-            val elements = source
             var avg: Any? = null
             var size = 1
 
-            for (element in elements) {
+            for (element in source) {
                 if (element == null) {
                     continue
                 }
@@ -36,7 +35,7 @@ object AvgEvaluator {
                         avg = element
                     } else {
                         ++size
-                        avg = AddEvaluator.add(avg, element)
+                        avg = AddEvaluator.add(avg, element, state)
                     }
                 } else {
                     throw InvalidOperatorArgument(