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

Date/time addition for partials is incorrect in some cases #1026

Open
brynrhodes opened this issue Sep 25, 2019 · 2 comments
Open

Date/time addition for partials is incorrect in some cases #1026

brynrhodes opened this issue Sep 25, 2019 · 2 comments
Labels

Comments

@brynrhodes
Copy link
Member

The following test:

define test: @2016 + 365 days

Returns @2016, when it should return @2017, based on the description of the operation in the spec. The reason is that the "time-valued quantity is converted to the most precise value specified in the date/time-valued argument". So 365 days would be converted to a year first, and then added. In other words, this is equivalent to @2016 + 1 year

@brynrhodes brynrhodes added the bug label Sep 25, 2019
@kyle-winkelman
Copy link

@brynrhodes
Question 1:
UCUM defines 1 year as 365.25 days, so when it is converted to years it ends up being 0.99931554 which is truncated away to be 0. Unless something other than UCUM conversion is expected to be used here. If not using UCUM conversions, it becomes quite complicated. For example @2016 + 365 days is not guaranteed to be @2017 (due to leap year), but @2017 + 365 days is guaranteed to be @2018.

Should the spec state how to to do the conversion?

Question 2:
The way I read the spec quantities should be converted immediately. @2016-01-01T00 + 1 year will result in @2016-12-31T06 (note the change of hours and falling short of adding a year due to being converted to 8766 hours).

Should the spec state:
"If the time-based quantity is more precise than the value specified in the first argument, the operation is performed by converting the time-based quantity to the most precise value specified in first argument (truncating any resulting decimal portion) and then adding it to the first argument."

Also the spec gives the below example which doesn't make sense unless the above clarification is made:
@2016-01-01 – 1.1 years will result in the value @2015-01-01 (1.1 years should be converted to 1.1 * 365.25 or 401.775 days)

Question 3:
Due to my understanding of the spec. I believe the AddEvaluator should convert the quantity before adding. This would mean the specific example you give as the bug above is correct but @2017 + 365 days should be @2017 but returns @2018 would be a new bug.

P.S. I am curious to know the proper forum for getting questions like these answered about the spec.

@brynrhodes
Copy link
Member Author

Apologies for missing this, but yes, for Question 1, that is currently an issue being addressed by an outstanding ballot comment reconciliation: J#27060.

For Question 2, yes, and that issue was addressed in the 1.5 ballot, as it now reads:

For partial date/time values where the time-valued quantity is more precise than the partial date/time, the operation is performed by converting the time-based quantity to the most precise value specified in the first argument

For Question 3, I'm not sure I'm following?

And for the P.S., since CQL is an HL7 specification, feedback can be submitted to the HL7 JIRA: http://jira.hl7.org. It requires an account, but the account is free to create. There is also an active CQL community within the FHIR/HL7 chat at http://chat.fhir.org.

Again, apologies for missing this.

@JPercival JPercival transferred this issue from cqframework/cql-engine Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants