Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncertainty Produced for Duration Operation Involving Days Granularity #325

Open
JSRankins opened this issue Sep 18, 2024 · 10 comments
Open

Comments

@JSRankins
Copy link

JSRankins commented Sep 18, 2024

Good afternoon. The MADiE team received a ticket related to uncertainty surfacing in a Measure Observation when a duration operation is invoked: MADIE-2024. It appears to occur when no milliseconds are provided by user during certain circumstances. To simplify things, I have created a small measure, test data sets and ran them through fqm-execution CLI 1.5.0. The data is attached.

In the test measure, the issue revolves around the following definition:

define "duration in days":
  ( singleton from ( "Inpatient Encounters" ) ) QualifyingEncounter
    return duration in days of QualifyingEncounter.period

Measure bundle: DMT-v0.0.000-FHIR.json
FHIR test case with milliseconds: DurationIssueMS.json
FHIR test case with no milliseconds: DurationIssueNoMS.json

Using the test with no milliseconds, the following comes back for the definition "duration in days":

{
  "libraryName": "DMT",
  "statementName": "duration in days",
  "localId": "19",
  "final": "TRUE",
  "relevance": "TRUE",
  "raw": {
    "low": 0,
    "high": 1
  },
  "isFunction": false,
  "pretty": "{\n  high: 1,\n  low: null\n}"
}

Using the test with milliseconds, the following comes back for the definition "duration in days":

{
  "libraryName": "DMT",
  "statementName": "duration in days",
  "localId": "19",
  "final": "TRUE",
  "relevance": "TRUE",
  "raw": 1,
  "isFunction": false,
  "pretty": "1"
}

Comments from @brynrhodes indicate that the duration calculation should not be producing an uncertainty in this instance. Please advise.

@cmoesel
Copy link
Member

cmoesel commented Sep 19, 2024

Hi @JSRankins and @brynrhodes. As you've noted, the data you've provided contains an interval with date/times that don't specify milliseconds. 5.5.5 Date and Time Intervals indicates that this should result in an uncertainty:

For example, the following interval expression contains all the instants of time, to the millisecond precision... However, if the boundaries of the interval are specified to a lower precision, the interval is interpreted as beginning at some time within the most specified precision, and ending at some time within the most specified precision... When calculating the duration of the interval, this imprecision will in general result in an uncertainty, just as it does when calculating the duration between two imprecise date or time values.

In your example, the imprecise 2026-11-12T13:41:38+00:00 represents the possibility of any millisecond 0 to 999, and the imprecise 2026-11-13T13:41:38+00:00 does the same. So there are cases in which the interval could be 0 days and cases where it could be 1 day (based on the set of all possible durations from all possible values of the imprecise date/times).

Is there somewhere in the specification that indicates that duration calculations should ignore milliseconds in this case?

@JSRankins
Copy link
Author

JSRankins commented Sep 19, 2024

@cmoesel , I think I understand what you are saying given the examples that are located in Appendix H: https://cql.hl7.org/15-h-timeintervalcalculations.html. It's possible within the range of possible milliseconds that the low and high together make the duration less than a whole day. The milliseconds are necessary for datetime calculation when the datetimes by themselves cannot be used to determine if a whole day has occurred. Since @DrMuir raised the concern based on what he was seeing, I'm copying here. @brynrhodes was previously included. @DrMuir @brynrhodes Any questions?

@brynrhodes
Copy link
Member

It's not that the calculation should ignore milliseconds, but rather that for the purposes of comparison, seconds and milliseconds are treated as a single precision represented as a decimal. This behavior is introduced in FHIRPath and discussed in the CQL specification in Comparing Dates and Times as well as in all the comparison operator descriptions, so I was thinking it would apply in determining duration as in this case. I think that could be interpreted either way though, since we are not explicit about it in the duration discussion. But also note that all the examples in the timing interval appendix use times specified to the second and are shown as giving integer results, not uncertainties. Thoughts?

@JSRankins
Copy link
Author

Thank you for the feedback @brynrhodes. If this is supposed to be based on FHIRPath, then looking at FHIRPath, it is clear that 2026-11-12T13:41:38.0 = 2026-11-12T13:41:38 . Refer to Date/Time Equivalence. If the engine is provided the following low and high, the result is 1 and not an uncertainty:

"period": {
  "start": "2026-11-12T13:41:38.0+00:00",
  "end": "2026-11-13T13:41:38.0+00:00"
}

That leads me to the belief that the engine should calculate using millisecond as 0 (instead of unknown) when seconds are provided and milliseconds aren't. This would resolve the issue. @cmoesel, thoughts?

@JSRankins
Copy link
Author

Just following up on this. @cmoesel, how do you want to proceed?

@DrMuir
Copy link

DrMuir commented Sep 27, 2024 via email

@cmoesel
Copy link
Member

cmoesel commented Sep 30, 2024

We still hold that the spec, as written, does not indicate that seconds and milliseconds should be combined for durations. It is clearly specified for comparisons, but calculating duration is not a comparison operator, so comparison semantics do not automatically apply. That said, based on conversation with @brynrhodes, I understand that the intent was for those same semantics to apply to duration, even if it was not clearly stated as such in the spec. But I think that Bryn also agreed that the specification fell short of specifying that clearly for durations.

Given that several here seem to agree that the desired behavior is for the seconds and milliseconds to be combined for duration calculations (like it is for comparisons), we would suggest that someone (@brynrhodes?) file a Jira issue to flag this as a technical correction and target a corresponding clarification for the upcoming CQL Errata release. Bryn can then bring this to the HL7 CDS Work Group for a vote to formalize agreement on the behavior. Once the CDS WG has voted to make this change to the spec in the Errata release, we can consider updating the JS engine to support it on time for the Errata publication.

@JSRankins
Copy link
Author

Thank you @cmoesel for your thorough analysis and response. @brynrhodes, do you want to submit the HL7 JIRA ticket to CQL? For now, I will update the fqm-execution ticket (Issue 312) to reflect status of this ticket: on hold for CQL update.

@brynrhodes
Copy link
Member

Thank you @JSRankins and @cmoesel , I've added this tracker here:

https://jira.hl7.org/browse/FHIR-48482

As part of putting this together, I did find these paragraphs at the end of the Determining Difference and Duration section:

Note that uncertainty calculations, just like date and time comparison calculations, consider seconds and milliseconds as a single combined precision with decimal semantics. For example:

hours between @2012-01-01T01:00:00 and @2012-01-01T02:00:00.0

The above example results in 1, even though the second argument is specified to milliseconds and the first argument is only specified to seconds.

Hopefully this helps, I've proposed a resolution for the tracker as well, and it would be ideal if we could discuss this at the CDS WG call tomorrow, though I understand that's pretty short notice :)

@cmoesel
Copy link
Member

cmoesel commented Oct 2, 2024

Thanks, @brynrhodes. I reviewed the Jira ticket and added a suggestion in the comments. It looks good to me and I will plan to join the CDS call today.

Also, thanks for finding that tidbit in the Language Semantics section of the spec. I must have missed that page when I was looking for guidance. I definitely looked at all the sections in the top-level menu (e.g., Author's Guide, Developer's Guide, ELM, Reference), but I guess I missed some of the "deep cuts"!

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

No branches or pull requests

4 participants