projecttacoma / fqm-execution

fqm-execution is a library that allows users to calculate FHIR-based electronic Clinical Quality Measures (eCQMs) and retrieve the results in a variety of formats
https://projecttacoma.github.io/fqm-execution/
Apache License 2.0
18 stars 6 forks source link

Birthdate Timezone #259

Closed jkotanchik-SB closed 1 year ago

jkotanchik-SB commented 1 year ago

Summary

The MADiE team has been researching some edge cases around birthdate for a user, most notably AgeInYearsAt(start of "Measurement Period"), and noticed that cql-execution is instantiating birthdate with a timezone offset equal to the user’s local UTC offset.

I was wondering if this was expected behavior as we have been passing in the Measurement Period to fqm-execution as UTC with the assumption that Dates, such as birthdate, would have an offset of 0.

External Tracking Ticket

Expected Behavior

When converting Birthdate to include a timezone, that timezone would have 0 offset.

Version or Commit

Inputs (e.g. Measure Bundle, Patient Bundle, CQL Library)

Relevant Calculation Options (e.g. Measurement Period, meta.profile Validation)

Measurement Period is passed with 0 offset timezones (UTC).

cql-to-elm Version Used for Measure Logic Translation (if known)

Any Additional Info

Attached Zip containing test CQL and test case JSON: Birthdate.zip

birick1 commented 1 year ago

My take is that this is expected behavior and the engine is evaluating the expression correctly.

Here are 4 quotes from the spec:

  1. CQL Appendix B: AgeAt: The AgeAt operators are defined in terms of a datetime duration calculation.

  2. CQL Appendix B: Duration: When this operator is called with both Date and DateTime inputs, the Date values will be implicitly converted to DateTime as defined by the ToDateTime operator.

  3. CQL Appendix B: ToDateTime: For the Date overload, the result will be a DateTime with the time components unspecified, except for the timezone offset, which will be set to the timezone offset of the evaluation request timestamp.

  4. CQL Author's Guide: However the evaluation occurs, the discussions in this chapter refer generally to the notion of an evaluation request that represents a request by some consumer to evaluate a CQL expression. This evaluation request generally includes the context of the evaluation (i.e. the inputs to the evaluation such as the patient and any parameter values), as well as a timestamp associated with when the evaluation request occurs.

Putting these quotes from the spec together:

This is what is being observed.

Moreover,

AgeInYearsAt(start of "Measurement Period"), and noticed that cql-execution is instantiating birthdate with a timezone offset equal to the user’s local UTC offset.

it's not that the birthDate is being instantiated this way. Rather, the Age operator is forcing an implicit DateTime conversion of birthDate that, by the spec, requires the timezone offset to be equal to the user's local UTC offset.

mgramigna commented 1 year ago

My take is that this is expected behavior and the engine is evaluating the expression correctly.

@birick1 I believe this is true except in a case where the "evaluation request timestamp" as mentioned in the spec is specifically set to have 0 timezone offset.

In fqm-execution, when parsing and setting the interval for the measurement period start and end, we explicity set the timezone offset to be 0 (the second argument in fromJsDate

In cql-execution, it seems that the intent is to respect the timezone offset of the evaluation request timestamp, which in this case should be 0 since fqm-execution is setting the execudtionDateTime parameter to have 0 timezone offset. We should confirm this intent with @cmoesel

@jkotanchik-SB To your original question, I believe in your case (will wait for confirmation), that the timezone offset of 0 is what actually should be used for the patient's birthDate, but I want to clarify that this is not because the offset matches the offset of the measurement period passed in, rather that fqm-execution always sets the execution timestamp to have a 0 timezone offset, and that is the offset that other datetimes in the execution context should respect.

cmoesel commented 1 year ago

I started to write a long response and then noticed that @birick1 already provided most of the analysis I was planning to walk through. Thanks for saving me some typing, Brian!

The one thing I'd note (as @mgramigna does above) is that, in practice, the "evaluation request timestamp" is not necessarily the offset of the computer doing the calculation. The execution environment can provide whatever evaluation request timestamp (with whatever offset) they want. This allows a computer in New York to evaluate CQL on behalf of a health system in California while respecting California's timezone.

So, if fqm-execution is passing in an executionDateTime with timezone offset 0, then I would expect ToDateTime(Date) to result in a DateTime with the correct year, month, and day, unspecified time units, and time zone offset 0.

I'll also not that the duration documentation also states:

When computing the duration between DateTime values with different timezone offsets, implementations should normalize to the timezone offset of the evaluation request timestamp, but only when the comparison precision is hours, minutes, seconds, or milliseconds.

So even if the offsets between the measurement period start and birthdate were different, we would essentially ignore the offsets anyway since the requested precision is in years. In our case, the offsets end up being the same (0), so it doesn't matter anyway -- but I guess it's worth noting that offset shouldn't come into the picture for any duration calculation in years (or months or weeks or days).

hossenlopp commented 1 year ago

Resolved via cql-execution bug fix released with cql-execution v3.0.1 and included in fqm-execution v1.3.0