smart-on-fhir / growth-chart-app

Other
67 stars 98 forks source link

Dates mismatch issue #35

Open shyamk26 opened 7 years ago

shyamk26 commented 7 years ago

For some patients, I noticed there is a dates mismatch in the height and weight results. After doing some research I figured its an issue with the date library used in the application. I also found a note in your Readme file which states:

Important

_The XDate library instance used in the app contains a custom fix related to the date use in windows opened by the app. (https://github.com/medapptech/smart_growth_charts/commit/56270b92f646484539ca272e4508daaec450c6d7) In the event that the library needs to be upgraded with a new version, this fix may need to be reapplied._

However,I am not able to find that commit on github. I also tried updating my code to the new Xdate library. But even that did not work. Can you please let me how this issue was fixed.

Thanks, Shyam

vlad-ignatov commented 7 years ago

Sorry for that. That link points to old repository that does not exist now but the fix was this: https://github.com/smart-on-fhir/growth-chart-app/blob/7be6e77cf77a4ff2a763a2470955aa1e268647cc/lib/xdate.js#L111-L115

I don't think that it is the reason unless you are observing the issue at the print-preview window. Can you tell us more about how to reproduce it? What are the FHIR server and the patient that you are using?

shyamk26 commented 7 years ago

Thank you for your quick response. We spotted this issue in our Prod domains and occurs only with a handful of patients. Please see the issue link below:

https://github.com/arshaw/xdate/issues/36

vlad-ignatov commented 7 years ago

OK. I think it might have something to do with the fact that months and years are flexible units. Do you have the same issue if you use days (or weeks, hours, seconds etc) instead of months?

shyamk26 commented 7 years ago

I tried changing the load-fhir-data.js from:

function months(d){ return -1 * new XDate(d).diffMonths(new XDate(p.demographics.birthday)); }

To: function months(d){ return -1 * new XDate(d).diffDays(new XDate(p.demographics.birthday))/30.4375; } But, the issue still exists. Is there anything else I can try?

vlad-ignatov commented 7 years ago

Can you describe how/where is the issue visualized?

vlad-ignatov commented 7 years ago

Also, I meant that this appears to work:

new XDate('2011-05-22').diffDays(new XDate('2016-03-01T15:26:00.000Z')) // -> 1745.4347222222223
new XDate('2011-05-22').addDays(1745.4347222222223) // -> XDate {0: Tue Mar 01 2016 10:26:00 GMT-0500 (EST)}
shyamk26 commented 7 years ago

We are actually modifying the code to use diffDays and addDays instead of months as you suggested. I will let you know if that solved the issue. However, I am sure others using this App will have issues with dates when they implement because of 'diffMonths' and 'addMonths'. Is there a fix your team is planning to roll out?

vlad-ignatov commented 7 years ago

New version of this app is currently being developed (but it will be commercial). They might decide to use different approach for that but the current version will not be fixed. You can try using days but unfortunately I have to say that this issue too fundamental to be fixed.

The primary functionality of this app is to “compare” patient data with standard data from predefined datasets. Those are the CDC, WHO, Fenton etc. datasets. For some reason, these datasets contain data points distributed by age expressed in months (and they don't use calendar months). Then we have a patient with certain DOB and number of observations.

So, if the FHIR server tells us that the patient weights 4kg 29 days after his DOB we need to convert those 29 days to months if we want to compare it with the dataset. If the DOB is 03/01 then 29 days are less than 1 month but for DOB = 02/01 it is more than a month...

Since the datasets are in months, the app does not have any other choice. It must use equalized months (something like 365/12). This was the only way we can have reliable computations. The side effect was that once we switch to those "absolute time intervals" it is sometimes impossible to convert them back to real calendar-aware intervals (just like you saw in the diffMonths example).

That is why I was asking you for better description of the issue. If it is a visualization issue, there is some small chance of success. If you manage to fix it we would appreciate a PR. Just keep in mind that those "absolute months" are critical for the app and it won't work correctly without them.

In other words, it is a delicate matter. If you are seeing incorrect ages, that might be fixed by computing them in a different way like XDate(DOB).diffDays(DOB).format(...). However, that MUST be done in such a way that does not modify the patient age in months as it is currently stored.