Skip to content

Update TruncatedDivide Quantity unit semantics to match plain Divide - #148

Merged
bryantaustin13 merged 1 commit into
cqframework:mainfrom
dehall:truncatedDivide-quantity-units
Sep 14, 2026
Merged

bryantaustin13 merged 1 commit into
cqframework:mainfrom
dehall:truncatedDivide-quantity-units

Conversation

@dehall

@dehall dehall commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This PR updates the expected results for TruncatedDivide tests on Quantity operands, to match the unit conversion semantics from plain Divide. Specifically, (from https://cql.hl7.org/09-b-cqlreference.html#divide )

performing division of quantities with the same units, the result will have the default UCUM unit ('1')

The spec doesn't explicitly call out this same note for TruncatedDivide, it only says (from https://cql.hl7.org/09-b-cqlreference.html#truncated-divide )

For division operations involving quantities, the resulting quantity will have an appropriate unit as determined by application of the UCUM specification.

But, the spec also says the same "appropriate unit [...]" for plain Divide, so if instead the semantics are intentionally different for TruncatedDivide vs Divide, I think that would need to be made more explicit.

For quick reference, here is the corresponding test for plain Divide

<test name="Divide1Q1Q" version="1.0">
<capability code="ucum-unit-conversion-support"/>
<expression>1'g/cm3' / 1'g/cm3'</expression>
<output>1.0'1'</output>
<!-- See http://unitsofmeasure.org/ucum.html#section-Semantics
particularly "the neutral element 1 (so called the unity)". -->
<!-- See HL7 STU comment #1442. -->
<!-- When dividing quantities with like units, the specification
should state clearly that the expected UCUM unit would be the
default unit (`1`). -->
<!-- See https://github.com/cqframework/clinical_quality_language/issues/193 -->

@bryantaustin13 bryantaustin13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Once merged, expect the reference engine to start failing, which is the point.

@cmoesel
cmoesel self-requested a review September 9, 2026 14:50

@cmoesel cmoesel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved (on behalf of MITRE's JS cql-execution team).

@bryantaustin13
bryantaustin13 merged commit 955a8f3 into cqframework:main Sep 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants