-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fixes Measure Evaluate Start and End times #362
base: master
Are you sure you want to change the base?
Conversation
…t day. Because it was not setting the precision, the default becomes SECONDS and when printing the report, it adds 00:00:00+00:00 without considering if the period was open or closed. By setting the precision correctly, it seems that the rest falls into place and the appropriate start and end times are considered. Expected: 2020-01-31T23:59:59+00:00 got: 2020-01-31T00:00:00+00:00
@JPercival looks like you changed these lines recently. You might be aware of this potential issue. |
We have a couple DateTime bugs in-flight, one was resolved with cqframework/clinical_quality_language#1259. I'm going to take another look at this once cqframework/clinical_quality_language#1269 lands. |
@Capt-Mac - Can you take a look at this? DateTime fixes were merged into CQL. Based on the spec here I'd say that the millisecond-level output is correct: |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #362 +/- ##
============================================
- Coverage 53.30% 53.30% -0.01%
Complexity 2133 2133
============================================
Files 281 281
Lines 11836 11840 +4
Branches 1618 1618
============================================
+ Hits 6309 6311 +2
- Misses 4912 4914 +2
Partials 615 615 ☔ View full report in Codecov by Sentry. |
@vitorpamplona I took a look at your changes to R4MeasureReportBuilder.java R4MeasureProcessor.buildMeasurementPeriod does correctly default interval to expected value, which is what the CQL will use to actually calculate results:
This matches spec definition https://build.fhir.org/operation-measure-evaluate-measure.html "The end of the measurement period. The period will end at the end of the period implied by the supplied timestamp. E.g. a value of 2014 would set the period end to be 2014-12-31T23:59:59 inclusive" MeasureReport.Period however has different requirements https://www.hl7.org/fhir/R4/datatypes.html#Period Though for clarity, I do think these values should match whatever value is passed in gets put on https://www.hl7.org/fhir/R4/datatypes.html#dateTime example: passed in => returned |
Something like this:
went from outputting
to
after the upgrade to 3.0.
After lots of debug, I believe the following lines set the precision to seconds without considering if the last day is closed or open, which causes the subsequent operations to be based on the 0 hour and the output to match that.