effected temp basal amounts #5740
Conversation
See issue #4971 for more information. #4971 * Insulin pump vendors vary in the timing between insulin dosings. * When modifying temp basal rates every 5 minutes, the differences between dose timings inherent in the pump delivery mechanisms become noticeable. * Timing issue is mitigated by creating a model for each pump and calculating the impact of each timing difference. * The result is a new piece of data: the effected insulin rate over an arbitrary period of time, eg 4 minutes and 57 seconds. * The new piece of data is uploaded via the `amount` parameter for Loop users as of September 2019. This patch favors the `amount` parameter when calculating effected temp basal rates.
Apply same patch for looking at effected temp basal amount preferentially through the `amount` field. This targets the text rendering of the amount printed on the basal line.
Apply same patch as others in line with checking for amount. See pull request #574 for more info. This patch should adjust the last treatment plugin to preferentially use amount of last treatment when available instead of `absolute` so that effected temp basal is accurately communicated.
|
I've audited all the places mentioned above and have questions about these two: TreatmentsShould default treatments that are uploaded be forced to have an https://github.com/nightscout/cgm-remote-monitor/blob/dev/lib/server/treatments.js#L169 Websocket QueryShould websocket be able to query by https://github.com/nightscout/cgm-remote-monitor/blob/dev/lib/server/websocket.js#L315 |
|
Sorry for comment lag - I'd very much prefer an approach where 1) we try to get the data formats unified across clients rather than supporting client quirks in the server (so this would be changed in Loop, not NS), and 2) if we support multiple data formats, we'd centralise data normalization to one part of the code such as the data loader, so the application logic only needs to know of one data format. If we go with the approach you have here (support multiple fields across the codebase) that easily leads to bugs, as devs would have to remember to use both fields at all times and it's then also a matter of time before someone adds a third field with the same data and adding support for that would be significantly easier if we have just one place where we add the field. Related, check the data formats specified in the V3 API; for maintainability it'd be great if we aim to not expand the feature set of the v1 api and modify the accepted data formats, vs encourage clients to migrate to v3. |
|
@ps2 would it be possible to have a look? |
|
Thanks for the good criticism @sulkaharo, I think I agree with your reasoning. 2.) I saw a place to do this during my audit, but it changes the logic in this patch, which depends on absence by default. |
|
@bewest Super happy to see someone tackling this! I'm not 100% sure I understand your question, but I can explain the reasoning behind sending the original scheduled For example: If Loop schedules a 1.2U/hr basal which ends up running for 5.5 minutes on an Omnipod, the pod will deliver 2 x 0.5 clicks, or 0.1U of insulin. If the timeline shows 1.09090909 U/hr (0.1U over 5.5m = 1.09090909 U/hr), instead of 1.2 U/hr, that's a very confusing experience. It's useful to have both pieces of information: the original scheduled rate, and the actual delivery. Reversing out the calculation of a rate from a delivery amount and time period produces numbers that can be unexpected and confusing for users. |
|
Also, an absolute "rate" of 1.09090909 U/hr is not even the actual rate of delivery happening then; that's just the average rate over those 5.5 minutes, so it's additionally misleading. I've made a lot of code updates in Loop to track these things separately. In retrospect (and I believe this statement applies to both code bases), it would have been better for the model to contain |
|
@bewest Thanks for working on this. I saw the comment from @sulkaharo I believe there is an underlying design issue that probably "grew" as NS reports were added with underlying assumptions that don't match some newer real-world use cases. (And I'm not trying to throw darts here - I completely understand how this happens.) I'm assuming that this is understood, but just in case, I'll belabor the point. There is no pump on the planet that provides continuous insulin drip and therefore calculating the amount of insulin delivered as rate times duration for a basal will always be an estimate. It is typically an overestimate for omnipod (because Insulet designed the pod to deliver its smallest increment at the end of a time unit - not the mid-point like Medtronic). I don't know about any other pump that might have data uploaded to Nightscout. I don't know if the folks supporting Omnipod for AAPS (or other instantiations for Omnipod) are uploading an Regarding the second comment: This is great - can we add into this that I think that a lot of the design of NS reports assume Thanks to all |
|
Apologies for the delay - #5980 now does the data normalization of the amount/absolute fields on runtime data load, which solves the immediate issue for every usage in the codebase except reports with less new logic overall. Note that in this PR, some of the changes would not work as the code is inside code blocks that are not reached if the |
#4971
.amountcontains effected rateImpacts nightscout reports for loop/omnipod users and becomes detectable comparing reports or even adding things up manually.
It looks to me like the issue in the daytoday reports traces through to line 369 in
getTempBasal.There are 18 other places that similarly use
.absoluteand conceivably some of them will need a similar patch or update to use a method to calculate effected rates. Thoughts?