-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Use full ICE history for displaying estimated carbohydrate effects #2163
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
The 'predicted' dataset on the 'glucose change' graph was calculated using insulin counteraction effects that had been truncated to the width of the chart. This meant that carb entries early in the timeline may not have dynamic carb absorption applied as the implied ICE was zero for the first part of the carb entry's history.
I cannot reproduce the issue reported in #2159 with my test phone. If there's an additional test that would be helpful, I could try again. |
Based on my understanding of the maths, to reproduce the issue prior to the patch you need:
|
I would like to try this, if I use your commt number and git hub info and add line to my yum file, would that work for browser build? |
Yes it would work (for dev branch, not for main branch). You can see the
lines you need in my browser build yum file.
…On Fri, 12 Jul 2024 at 3:57 AM, Trpl7ca ***@***.***> wrote:
I would like to try this, if I use your commt number and git hub info and
add line to my yum file, would that work for browser build?
—
Reply to this email directly, view it on GitHub
<#2163 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFRGMJ5Y35V2ZCDEY7J7OTZL3BPDAVCNFSM6AAAAABHDS4YQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRTGU2DKNJQGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not try to reproduce, but this simple change seems reasonable and safe to do with my limited understanding of this code.
This code will change with the tidepool-merge (and I think this display bug may not exist in that branch, from reading the code - I have not tested tidepool-merge) |
I did test tidepool-merge (if you look at LoopWorkspace PR 513, you will see some testing summary comments.) We plan a new release from dev before merging tidepool-merge. Whether this PR is merged as part of that process is under discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is already fixed in the tidepool merge. We can merge this into dev for now, since it looks like it should fix the issue there, and I'll just have to resolve the conflicts before tidepool-merge lands.
The 'predicted' dataset on the 'glucose change' graph was calculated using insulin counteraction effects that had been truncated to the width of the chart. This meant that carb entries early in the timeline may not have dynamic carb absorption applied as the implied ICE was zero for the first part of the carb entry's history.
Addresses the different behaviour in landscape and portrait view of the glucose change graph in the carbohydrate screen. so that dynamic carb effects are always shown when they are in effect.
For discussion (but not included in this pull request), for clarity the graph dataset label could be changed from 'Predicted' to 'Estimated and predicted' given that it already incorporates dynamic glucose effects (when these are in effect), so is partly based on observation.
Does not address what I described as 'desired' behaviour of showing a static prediction for the 'predicted' dataset; I am not sure how you would achieve that.
Closes #2159