tidepool-org / hub

[DEPRECATED] Central storage for Tidepool planning and issue tracking.
2 stars 2 forks source link

add `units` property to `insulinSensitivity` component objects #86

Open jebeck opened 10 years ago

jebeck commented 10 years ago

insulinSensitivity is expressed in mg/dL(/U) or mmol/L(/U), so we need to know which we're looking at!

ETA: Ah, now I realize we do have a units property that's settings-wide, although it's specified as referring to BG, which is not its only application here, so either we should find a new way to express it or be redundant...?

cheddar commented 10 years ago

insulinSensitivity is the number of points of BG you expect to go down to one unit of insulin right? How is that not the same unit as BG?

jebeck commented 10 years ago

Maybe I should backtrack to the high-level goal I'm trying to solve here. Right now Sara's BG data get converted to mg/dL, but this particular bit of data insulinSensitivity slips through the cracks. I'd like to have a way of easily identifying objects that may be subject to conversion or, down the road, different treatment (formatting-wise) because we're no longer converting them, just supporting them. It seems to be that an easy way to identify such things would be if they all have units specified as a property. Does that makes sense @cheddar? I'm open to suggestions.

cheddar commented 10 years ago

Ok, that does make sense.

Fwiw, contrary to what I was first thinking, I'm now thinking that we probably want to standardize on one unit to store in the platform and have a global toggle on the display to switch into something else.

In this world, you would be able to assume that all bg thingies are a specific unit and convert when displaying. Does that change this at all?

jebeck commented 10 years ago

@cheddar I disagree fairly strongly on the point of storing in one unit. The issue, as I see it, is the following:

Assumption 1: as stewards of diabetes data, it is our duty to display to the user the same datum they would have seen on their device - 109 mg/dL should appear as 109 mg/dL, and 5.8 mmol/L should appear as 5.8 mmol/L.

Assumption 2: You'd propose converting to and storing in mg/dL.

The problem here is that we can't, strictly speaking, make this conversion. In order to convert back to mmol/L with enough fidelity to satisfy Assumption 1, we'd have to store the converted original mmol/L in "mg/dL" to some number of decimal points of accuracy that actually doesn't make sense in the world of mg/dL units, which as far as I've ever seen them in meter or sensor data, are integer values.

Maybe it also helps to explain that the situation you describe (storing in one unit) actually requires more work in blip and/or visualization code than the other? Both versions require factoring out the parts of the display that are sensitive to units - i.e., the places where we spell out 'mg/dL' in labels, etc. Storing in different units requires no additional code to construct the scales - these are dynamically calculated based on the min and max of the data, and the units don't matter at all. (The formatting of the axis ticks (as integers vs. decimals) does matter, but D3 keeps axes nicely separate from scales, and the formatting of the axis ticks falls under the umbrella of things that will need a bit of refactoring either way.) Storing in one unit would add the additional step of converting to the display unit before visualization could take place.

All this said, I can see the use case for allowing the user to change the units in which their data are displayed, but I think we'd be better off storing the values exactly as we get them and doing conversion on demand client-side.

kentquirk commented 10 years ago

If we're storing data as numbers rather than strings, then we should standardize on a unit and convert. In Javascript, integers don't exist -- everything is a Number. The problem is in how you display that number. With proper rounding logic, we should never see floating point artifacts; at the scales we're talking about, any errors will be something like 5-7 digits down.

If it's really useful to store the values "exactly as we get them" we should be storing strings, not numbers. Maybe we should do both -- store the original string, and a numeric value.

I think consumers of our API will expect that all users will have their data available in the same format. To store the values on a per-user (or worse, per-device) basis will cause innumerable bugs down the road. I actually think that in the long run we will want to allow a configuration on the API setup call and let the API do the conversion -- that way, client apps can do what you suggest and not worry about the units, because they're set by the API in advance.

For now, though, I think we should store a single data type. I'd suggest that we store mg/dL as a floating point value (note that if we're given mmol/L, we have to convert to mg/dL and we should NOT truncate or round it). We then convert to mmol/L when necessary, and round (not truncate) to the nearest integer for mg/dL, and a single decimal point for mmol/L. I wouldn't object to also storing the "original" value as a string for reference purposes.

jebeck commented 10 years ago

I am happy with storing the original value as a string. I feel strongly that we need to be able to test and/or validate our (potentially) twice-converted value against the original to be confident that we have not altered it. As long as we have the original string as the means to do that, I'll be satisfied.