Repository navigation
Add quantity unit tests for multiply and divide (FHIR-58137) - #161
Open
bryantaustin13 wants to merge 6 commits into
Open
bryantaustin13 wants to merge 6 commits into
bryantaustin13 wants to merge 6 commits into
Conversation
Covers the examples in cqframework/clinical_quality_language#1690 and the average daily dose calculation from CMS108: mg / d, mg / mL, mg / dL, mg/mL * mL, mg/d * d, plus the spec's own 12 'cm2' / 3 'cm' and 3 'cm' * 12 'cm2' examples. Two equality tests check the value independently of how the quotient's unit is written. The expected units follow the spec examples and the cql-execution approach discussed in #1690; the general unit rule is pending https://jira.hl7.org/browse/FHIR-58137. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the proposal examples not already covered: 120 'm' / 60 's', 10 'm/s' * 10 's', 2 'm' * 3 'cm', 2 'm' / 3 'cm' and 4 'm' / 2 'cm'. Also adds 10 'mg/mL' * 5 'L', which applies the most-granular-unit conversion to a dosing calculation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
This rule would also settle an existing test that currently fails for the same reason. |
Adds Paul Lynch's three cases where a unit survives in the divisor: 10 'm2' / 2 'cm', 10 'cm2' / 2 'm', and 10 '/cm' / 2 'm'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Per FHIR-58137 discussion, granularity considers the exponent sign, so '/m' is more granular than '/cm'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # tests/cql/CqlArithmeticFunctionsTest.xml
The merge of main into this branch (b62e7ca) kept this branch's stale copies of tests that main had since changed: - Exp1000, Exp1000D, Ln0, LnNeg0, PredecessorUnderflowDt, PredecessorUnderflowT, SuccessorOverflowDt and SuccessorOverflowT went back to invalid="true"; main expects null. - FloorIntegerGreaterThanMaxInteger and FloorIntegerLessThanMinInteger lost invalid="semantic". Rebuilt the file as main plus this branch's own additions, so the diff against main is only the new quantity multiply and divide tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test cases for quantity multiplication and division units, covering the examples in cqframework/clinical_quality_language#1690 and the proposal in FHIR-58137. The committees asked for these tests so the proposed language can be checked for aligned implementations before the ITS call on 10/7.
The motivating case is the CMS108
averageDailyDosecalculation. Dividing a total dose by days supplied should givemg/d, but the Java engine converts to UCUM base units and truncates to 8 decimal places, so the measure had to build'mg/d'by hand.All tests are in
CqlArithmeticFunctionsTest.xml, in the existingDivideandMultiplygroups.Stated directly in the CQL spec
Divide12CM2By3CM12 'cm2' / 3 'cm'4.0 'cm'Multiply3CMBy12CM23 'cm' * 12 'cm2'36.0 'cm3'These are the examples in Divide and Multiply, present since 1.4.
Proposed by FHIR-58137
Units of the same dimension convert to the most granular unit, then the units combine by adding exponents (subtracting the denominator's for division). Atoms with an exponent of zero are removed, and
'1'is used when none remain.Divide10MGBy2D10 'mg' / 2 'd'5.0 'mg/d'Divide10MGBy5ML10 'mg' / 5 'mL'2.0 'mg/mL'Divide1MGBy1DL1 'mg' / 1 'dL'1.0 'mg/dL'DivideAverageDailyDoseTablets(60 '1' * 10 'mg') / 30 'd'20.0 'mg/d'DivideAverageDailyDoseSolution(150 'mL' * 2 'mg/mL') / 30 'd'10.0 'mg/d'Divide120MBy60S120 'm' / 60 's'2.0 'm/s'Divide4MBy2CM4 'm' / 2 'cm'200.0 '1'Divide2MBy3CMRound((2 'm' / 3 'cm').value, 8)66.66666667Divide10M2By2CM10 'm2' / 2 'cm'50000.0 'cm'Divide10CM2By2M10 'cm2' / 2 'm'0.05 'cm'Divide10PerCMBy2M10 '/cm' / 2 'm'500.0 '/m2'Multiply10MGPerMLBy5ML10 'mg/mL' * 5 'mL'50.0 'mg'Multiply5MGPerDBy2D5 'mg/d' * 2 'd'10.0 'mg'Multiply10MPerSBy10S10 'm/s' * 10 's'100.0 'm'Multiply2MBy3CM2 'm' * 3 'cm'600.0 'cm2'Multiply10MGPerMLBy5L10 'mg/mL' * 5 'L'50000.0 'mg'The last three division tests were added from Paul Lynch's review comment on FHIR-58137. They cover division where a unit remains in the divisor.
Independent of how the unit is written
Divide10MGBy2DEqual(10 'mg' / 2 'd') = 5 'mg/d'trueDivideAverageDailyDoseEqual((60 '1' * 10 'mg') / 30 'd') = 20 'mg/d'trueThese hold whatever unit the quotient is written in, as long as the value isn't truncated (Equal).
Current results
All 20 tests validate against
tests/testSchema.xsd. Against the cqframework Java engine (5.3.0), 2 of the first 17 pass and 15 fail. For example,10 'mg' / 2 'd'returns5E-8 'g.s-1'(about 4.32 mg/d), the tablet dose returns2.3E-7 'g.s-1'(19.87 mg/d instead of 20), and2 'm' / 3 'cm'returns70. I haven't run these against cql-execution.The three divisor tests, run later (2026-10-29) through cql-tests-runner against the Java engine on HAPI FHIR 8.12.0, all fail. In each case the engine's value is the same quantity as the expected one, but the unit differs:
Divide10M2By2CM50000.0 'cm'500 'm'Divide10CM2By2M0.05 'cm'0.0005 'm'Divide10PerCMBy2M500.0 '/m2'500 'm-2'The first two convert to the base unit
m, which is what FHIR-58137 addresses. The third is open question 1 below.Open questions for the ITS call
Divide120MBy60Sfails on the Java engine only because it returns'm.s-1'instead of'm/s'. Both are valid UCUM for the same unit, but the runner compares unit codes as exact strings. Either the spec should pick a written form, or the runner should compare units for UCUM equivalence.Divide10PerCMBy2Mshows the same thing: the engine picks the agreed unit but writes it'm-2', where the test expects'/m2'.10 'mg' / 2 days.version="1.0"to match the existing groups. If the proposal goes into a specific CQL release, all but the two spec-example tests should probably use that version.m-2is finer thancm-2).Divide10PerCMBy2Mtherefore expects500 '/m2'. Special units (UCUMspecialunits such asB[V]) are excluded from the granularity comparison. Operations on them already return empty, so this only affects comparisons.🤖 Generated with Claude Code