tidepool-org / deprecated-data-model

[DEPRECATED] refer to https://github.com/tidepool-org/tidepool-org.github.io
Other
1 stars 3 forks source link

How to store datetime for data from devices that are not timezone aware #1

Closed nicolashery closed 10 years ago

nicolashery commented 10 years ago

This is to continue my discussion with @jebeck and @cheddar. I needed a place to write code. It's more illustrative I find :)

The suggestion that follows is certainly not bullet-proof, but hopefully it's future-proof enough, and could be a good place to start.

The thing I want to avoid, is needing data from another part of the system (say a timezone offset on the user object), to be able to display the "localDateTime" of a device data record correctly. I need to be able to do it from the record's JSON directly.

// NOTE: You can try these out by opening a JS console on the page
// http://momentjs.com/timezone/docs/

var DATETIME_FORMAT = 'YYYY-MM-DDTHH:mm:ss';

// Let's consider a diabetes device reading (device is not timezone-aware)
// I want to be able to display what was on the device when it happened
// (what the user saw), let's call it "localDateTime"
// In this example, it's 14:51 (on 2014-01-31)

// I synced my data in Europe, but it's data that was created when I was back 
// in California
// It gets recorded below, with a Europe timezone offset (UTC -60 minutes)
// The UTC is "wrong", but that's fine, I can still get what I want to display,
// i.e. 14:51

// QUESTION 1: How do I store readings of my data? A couple options...

// Much info
var reading = {
  utcDateTime: '2014-01-31T13:51:09',
  // localDateTime: Redundant info, but might be useful for sorting
  localDateTime: '2014-01-31T14:51:09',
  timeZoneOffset: -60,
  timeZoneName: 'Europe/Berlin'
};

// Medium info
var reading = {
  utcDateTime: '2014-01-31T13:51:09',
  timeZoneOffset: -60,
  timeZoneName: 'Europe/Berlin'
};

// Minimum info
var reading = {
  utcDateTime: '2014-01-31T13:51:09',
  timeZoneOffset: -60
};

// QUESTION 2: How do I display the reading correctly,
// i.e. that 14:51 that was showing on the device when the reading happened.
// I alse want it to display correctly wether I'm using a browser in California 
// or Europe

function getLocalDateTimeFromUtc(item) {
  var m = moment.utc(item.utcDateTime);
  m.zone(item.timeZoneOffset);
  return m.format(DATETIME_FORMAT);
}

function getLocalDateTime(item) {
  if (item.localDateTime) {
    return item.localDateTime;
  }
  return getLocalDateTimeFromUtc(item);
}

getLocalDateTime(reading); // => '2014-01-31T14:51:09'

// QUESTION 3: A new feature allows me to "correct" the timezone info, 
// reading by reading
// How do I do it, while safely keeping that precious "localDateTime" 
// value (14:51)

// Option 1: with timezone offset. Not ideal, because Daylight Savings
function changeTimeZoneOffset(item, newTimeZoneOffset) {
  // Keep local date time, change UTC to reflect new tz offset
  var shiftUtcByMinutes = newTimeZoneOffset - item.timeZoneOffset;
  var m = moment.utc(item.utcDateTime);
  m.add('minutes', shiftUtcByMinutes);
  item.utcDateTime = m.format(DATETIME_FORMAT);
  item.timeZoneOffset = newTimeZoneOffset;
  return item;
}

// I know California on that day was UTC +480 minutes
// (but that's not always the case with Daylight Savings)
changeTimeZoneOffset(reading, 480);
// => Object {utcDateTime: "2014-01-31T22:51:09", timeZoneOffset: 480}
// And still displays correctly:
getLocalDateTime(reading); // => '2014-01-31T14:51:09'

// Option 2: with timezone name. Ideal, should handle Daylight Savings
// http://momentjs.com/timezone/docs/
function changeTimeZoneName(item, newTimeZoneName) {
  var m = moment.utc(item.utcDateTime);
  // What time was it in the new timezone?
  m.tz(newTimeZoneName);
  // Now I have the new timezone's offset
  var newTimeZoneOffset = m.zone();
  item.timeZoneName = newTimeZoneName;
  return changeTimeZoneOffset(item, newTimeZoneOffset);
}

// I know California is *always* timezone 'America/Los_Angeles'
changeTimeZoneName(reading, 'America/Los_Angeles');
// => Object {utcDateTime: "2014-01-31T22:51:09", timeZoneOffset: 480, 
// timeZoneName: "America/Los_Angeles"}
// And still displays correctly:
getLocalDateTime(reading); // => '2014-01-31T14:51:09'

// How did I get the timezone name when syncing data?
// https://bitbucket.org/pellepim/jstimezonedetect
function determineUserTimeZoneName() {
  var timeZone = jstz.determine();
  return timeZone.name()
}

// When I synced data in Europe, my browser gave:
determineUserTimeZoneName(); // => 'Europe/Berlin'

// NOTE: Using moment, I always call moment.utc('...'), and never moment('...')
// because moment always creates a date in the browser's timezone, except if 
// I use moment.utc('...')
cheddar commented 10 years ago

My short response to this is that I agree that timezone should be on each datum and I think that in the short-term, we should ask for timezone on upload and store it along with all of the data points that were provided in that upload. We should default that timezone selection to the timezone that the person is in (figured out however, I don't care)

After we go to UCSF, I see us spending the time to make it so that you aren't deleting your entire data set every time you upload. Which should mean that we can provide an experience where someone could chunk up their data and upload it in separate blocks providing the correct timezone for each. There are then other optimizations that can be done to improve the user experience from there, but I think this is the only way we will be able to get correct timezones associated with the data.

kentquirk commented 10 years ago

Ok, I'm not understanding something.

I thought the whole conversation we had earlier this week was that, for the pilot, what you're calling "localDateTime" (I thought we had settled on "deviceTime" but whatever) was going to be stored without time zone information, since we don't know the time zone, and that we would persist it that way and display it that way. In other words, there would be NO ATTEMPT to adjust this time. If it was recorded as 14:51, it gets saved as 14:51 and only ever displayed as 14:51.

So I don't really understand what you're getting at here. If you're reintroducing the concept of adjusting device data to some time zone, we need to open up that entire conversation again. @jebeck?

In the long run, I believe this strategy of timezoneless information can't and won't persist. We ARE going to have to attach time zone information to time even if device makers are late to the party, and we ARE going to have to have a flexible, easy-to-use tool for selecting and time-adjusting blocks of a patient's data, which will be a challenging but achievable UI problem. However, for the short term we can get away without it, and even in the long term it doesn't need to be part of blip.

jebeck commented 10 years ago

@kentquirk You understood this week's discussion correctly, and 'localDateTime' == 'deviceTime'.

This is just continuing the discussion, more or less, not talking about the pilot. @cheddar still had some questions, but (and correct me if I'm summarizing incorrectly) agreed to compromise on the deviceTime solution for the pilot.

As discussed with you and @jh-bate, timezone information will come into play when displaying messages, but that's the only place where they will.

kentquirk commented 10 years ago

So, why, then, is the code above all about messing with and storing time zones? Oh, you said "not about the pilot". If that's the case, I'd REALLY like to postpone this discussion.

cheddar commented 10 years ago

I'm in disagreement with the decision to ignore timezone for the pilot and Jana/Nico have been interested in why that is.

I've been explicit with them that I'm willing to agree to disagree about maintaining timezones for the pilot, I'm not trying to change that decision, but there's been an email discussion that got moved here because Nico wanted to be able to write some code ideas. That's the context. Will forward you email thread as well.

jebeck commented 10 years ago

@cheddar I'm curious about your long-term solution: "Which should mean that we can provide an experience where someone could chunk up their data and upload it in separate blocks providing the correct timezone for each."

I don't understand how this will be possible from a usability perspective. You're asking the user to accurately segment their data into single timezone chunks before upload? What would the UI for this be? Why would a user care to go through this tedious step? Keep in mind the break point between timezones has to be accurate - it can't just be, oh, I was traveling in EST 1/3-1/9. We'll have to know the exact time when the user changed their timezone on each of their devices in order to display all the data correctly.

I still believe the only long-term solutions are (1) device makers provide ISO 8061 timestamps with full UTC offset information or (2) for the devices where we can (i.e., Medtronic, Dexcom), we bootstrap from a combination of verifying the timezone for the most recent data when the user is uploading plus the history of date/time settings changes (Medtronic) or continually inspecting the offset between the user timestamps and device-internal timestamps (Dexcom).

brandonarbiter commented 10 years ago

A request came through from Helmsley Charitable Trust and JDRF that we build a "time zone correction" application (or feature) to be used at the clinic. It's not about "time zone" per se, but in case it's relevant to this thread I want to provide.

It might work like this:

  1. Clinician uploads device data on behalf of patient
  2. Clinician realizes timestamp is off by 6 hours and 32 minutes.
  3. Clinician gets permission from patient to adjust timestamp
  4. Clinician uses Tidepool app, selects a period of data (perhaps 6 weeks), and updates the time
  5. Then the clinician and the patient review the updated data together in a Tidepool (or other) app

Brandon

Brandon Arbiter VP, Product + Biz Dev Tidepool http://www.tidepool.org | brandon@tidepool.org 917.536.0505 (m)

On Fri, Jan 31, 2014 at 11:19 AM, Jana Beck notifications@github.comwrote:

@cheddar https://github.com/cheddar I'm curious about your long-term solution: "Which should mean that we can provide an experience where someone could chunk up their data and upload it in separate blocks providing the correct timezone for each."

I don't understand how this will be possible from a usability perspective. You're asking the user to accurately segment their data into single timezone chunks before upload? What would the UI for this be? Why would a user care to go through this tedious step? Keep in mind the break point between timezones has to be accurate - it can't just be, oh, I was traveling in EST 1/3-1/9. We'll have to know the exact time when the user changed their timezone on each of their devices in order to display all the data correctly.

I still believe the only long-term solutions are (1) device makers provide ISO 8061 timestamps with full UTC offset information or (2) for the devices where we can (i.e., Medtronic, Dexcom), we bootstrap from a combination of verifying the timezone for the most recent data when the user is uploading plus the history of date/time settings changes (Medtronic) or continually inspecting the offset between the user timestamps and device-internal timestamps (Dexcom).

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/data-model/issues/1#issuecomment-33808073 .

cheddar commented 10 years ago

I agree that the initial experience is less than good. I think some things like what Brandon is talking about are possible. I also don't know that now is the right time to discuss this.

HowardLook commented 10 years ago

I think of “user specified timezone” as an example of a transformation of the data. So is “applied time offset” (because time was off) or “adjusted for daylight savings time” or “added +2 mg/dl to compensate for known calibration error” or “display time in my local time zone, no matter where it was collected” (for telemedicine).

I think that we should never assume that user-specified things are correct. They are transformations to the original data from the device. I think you store the original data, then store all of the transformations (with signatures so we can verify that they are correct).

H

On Jan 31, 2014, at 7:57 AM, Kent Quirk notifications@github.com wrote:

Ok, I'm not understanding something.

I thought the whole conversation we had earlier this week was that, for the pilot, what you're calling "localDateTime" (I thought we had settled on "deviceTime" but whatever) was going to be stored without time zone information, since we don't know the time zone, and that we would persist it that way and display it that way. In other words, there would be NO ATTEMPT to adjust this time. If it was recorded as 14:51, it gets saved as 14:51 and only ever displayed as 14:51.

So I don't really understand what you're getting at here. If you're reintroducing the concept of adjusting device data to some time zone, we need to open up that entire conversation again. @jebeck?

In the long run, I believe this strategy of timezoneless information can't and won't persist. We ARE going to have to attach time zone information to time even if device makers are late to the party, and we ARE going to have to have a flexible, easy-to-use tool for selecting and time-adjusting blocks of a patient's data, which will be a challenging but achievable UI problem. However, for the short term we can get away without it, and even in the long term it doesn't need to be part of blip.

— Reply to this email directly or view it on GitHub.

kentquirk commented 10 years ago

Ok, @cheddar let me know there was more here than I'd been involved with.

So I believe he's correct in the long run -- we MUST have a time zone for every event we're tracking. Every event must have an "absolute" time at which it occurred. It's the only way to achieve sanity.

The result of that decision is that we must eventually (as @brandonarbiter notes) have a time adjustment tool. When I use the word "transformations" that's mainly what I've been talking about. As I said, it won't be easy to make it friendly and usable, but I do believe that it will be possible to do so (and have actually been thinking about this and planning for it for several months, and have even had creative conversations about the design with various people).

We've also decided for the short term (the pilot) that we're going to ONLY display device-recorded time without time zone. So, just to close the question I had from the first message -- please don't reintroduce time zone manipulation as part of the blip pilot.

bewest commented 10 years ago

Future will be different from short term.

With my changes we will be tracking all raw data, and are free to re-ingest it at any time. In the short term we have a kludge which wipes out all your old data from the db, and inserts only new data. In the future I will provide a mechanism to merge new data with old data. The same mechanism which provides this feature also makes it possible for users to correct data safely.

Eg: let me export data from eg "the second week for July", edit it in google docs, and then re-import it with fixes. When the user sends us the updated spreadsheet for that week or whatever, we will also keep a log of the differences and why the user wanted to make that change -- forever -- along with the original raw data.

But we don't need all of that for UCSF pilot... give me a few more weeks after we're ready for pilot, please. I have a bunch of technical notes scattered across the web to validate this approach, since this is something I've been preparing for years called "git-phr", please let me know if you'd like to see them.

Short term for pilot:

Regardless of what timezone information chooses to use or consume or store, parsers should never adjust values of raw data, they should simply report what they observe. Eg if we decide later to store different timestamps, the parsers will not need modification (because blip will add that information after the data has been parsed). My point all along is simply that it's not the job of the parsers. Blip or blip-data-api can certainly amend the data for visualization either before it goes in the db and after parsing or after fetching from the db but before sending to the visualization.

-bewest

On Fri, Jan 31, 2014 at 8:33 AM, Kent Quirk notifications@github.comwrote:

Ok, @cheddar https://github.com/cheddar let me know there was more here than I'd been involved with.

So I believe he's correct in the long run -- we MUST have a time zone for every event we're tracking. Every event must have an "absolute" time at which it occurred. It's the only way to achieve sanity.

The result of that decision is that we must eventually (as @brandonarbiterhttps://github.com/brandonarbiternotes) have a time adjustment tool. When I use the word "transformations" that's mainly what I've been talking about. As I said, it won't be easy to make it friendly and usable, but I do believe that it will be possible to do so (and have actually been thinking about this and planning for it for several months, and have even had creative conversations about the design with various people).

We've also decided for the short term (the pilot) that we're going to ONLY display device-recorded time without time zone. So, just to close the question I had from the first message -- please don't reintroduce time zone manipulation as part of the blip pilot.

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/data-model/issues/1#issuecomment-33809488 .