Separate absorptionTimeOverrun into two variables - expectation and expiry

The variable absorptionTimeOverrun currently modifies the user inputted carb absorption time by multiplying it by 1.5. This serves multiple purposes which different users might want to manage separately from each other:

Expiring carbs when they never show up. For these purposes, using a longer time than the user's input is the natural choice. No change needed.
Reducing upfront bolus for slow burning meals to avoid lows. This is largely addressed by the implementation of Dosing algorithm #533. Even with that implemented, some people might feel that for large, complex, slow-burning meals and their associated uncertain absorption time, they prefer a smaller bolus which results from extending the absorption time. This can be achieved extending the inputted CA in those particular cases, or for users who want to always be conservative in this regard, the code could be modified.
For retrospective correction, loop compares actual outcomes to expectation. For these purposes, modifying the user's actual expected CA seems to conflict with the underlying concept behind RC, and might produce unnecessary high temping.

Suggestion is to have the prediction be based on the expected absorption rate derived from the entry absorption, not the minimum absorption rate, as I believe it currently is, while continuing to expire carbs based on the minimum absorption rate. Adopting this suggestions would result in higher boluses for very slow absorbing meals, but these should be accurate/desirable, based on #533. And it would tend to strengthen the positive feedback loop between accuracy of user CA input, ex-post review in the carb absorption detail screen, and retrospective correction activity.

@jeremybarnum

Hi @jeremybarnum, I've not forgotten about this. I agree with your suggestion and I'm planning to go through the dynamic carb code to see if I could make (hopefully small) modifications to test out decoupling the initially expected absorption rate from the minimum absorption rate. If you've already done anything along these lines, let me know.

@dm61

Thanks @dm61 that's great to hear. Yes, I have tried, and while I don't want to confess how long it took me to actually figure out what changes were needed, I think I have a plausible starting point - limited by my general coding ignorance. And it does seem that the changes are small. I was waiting to post it for my extra RileyLink to arrive so I could properly test it. But since you asked...I think it starts here.

/// The maximum amount of time needed for the remaining entry grams to absorb, at the fixed 
/// minimum absorption rate
    private var estimatedTimeRemaining: TimeInterval {
        guard minAbsorptionRate > 0 else {
            return 0
        }
        return (entryGrams - clampedGrams) / minAbsorptionRate
    }

I believe the comment is misleading because it suggests that the purpose here is to compute the maximum time needed - but in effect, it is this variable estimatedTimeRemaining that seems to drive the absorption time used to generate the carb effect vector for the remaining unabsorbed carbs, which in turn drives the prediction. The carb effect generation starts here where the function dynamicAbsorbedCarbs() is defined, and here is the key part:

            guard let end = observedTimeline.last?.endDate, date <= end else {
            // Predicted absorption for remaining carbs, post-observation
            let total = absorption.remaining.doubleValue(for: unit)
            let time = date.timeIntervalSince(absorption.observedDate.end)
            let absorptionTime = absorption.estimatedTimeRemaining

            return absorption.clamped.doubleValue(for: unit) + LinearAbsorption.absorbedCarbs(of: total, atTime: time, absorptionTime: absorptionTime)
        }

So it seems that in order to pass the remaining time into the function that generates the carb effects, we need estimatedTimeRemaining to be computed using the expected absorption rate, not minimum absorption rate. There doesn't seem to be a variable for expected absorption rate, so I guess we should create it in the same place we create minimum absorption rate, and it should be

    /// The expected carb absorption rate for this entry, in g/s
    var expectedAbsorptionRate: Double {
        return entryGrams / entry.AbsorptionTime
    }

Then we would replace this line

return (entryGrams - clampedGrams) / minAbsorptionRate

with something like this

return (entryGrams - clampedGrams) / expectedAbsorptionRate

So far I've built sucessfully with that modication by doing the local repository thing with cartfile to incorporate changes to loopkit. But without getting a prediction running I can't know if it's working and I haven't wanted to hijack my wife's live hardware, so I got another RL for testing. What I can see is that the carb decay graph still seems to be extended by the absorptionTimeOverrun but maybe this is just a UI thing? Anyway, with the right eyes on it I'm sure this will be clarified quickly!

Then after this is figured out there is the question of whether anyone wants this change! people seems to be getting really good results even though RC is currently a little "wrong". But at least we'll have a clear path to doing the modification for those who want it, if we can figure this out.

Great, thanks for providing starting points.

@dm61

@dm61 Update: I'm now set up to test, and this doesn't work. Actually when I run carthage update --platform iOS LoopKit it won't build. I thought I had built successfully before with these changes and I just had to test whether the prediction made sense, but I guess I hadn't. I think this is a basic sort of coding/Swift error (not unwrapping variables properly?), but I'm out of my depth here. Need help from people with skills! Here is the terminal output:

MacBook-Pro-4:loop Jeremy$ carthage update --platform iOS LoopKit
*** Fetching Amplitude-iOS
*** Fetching SwiftCharts
*** Fetching LoopKit
*** Fetching rileylink_ios
*** Fetching G4ShareSpy
*** Fetching dexcom-share-client-swift
*** Fetching xDripG5
*** Checking out LoopKit at "0c94d6e9b4e344d5fa10db7b8bf9ed942292e259"
*** xcodebuild output can be found in /var/folders/7z/pgk4zl0138v0hswv4p4fjft80000gq/T/carthage-xcodebuild.8yzMqs.log
*** Building scheme "LoopKit" in LoopKit.xcodeproj
*** Building scheme "InsulinKit" in LoopKit.xcodeproj
*** Building scheme "CarbKit" in LoopKit.xcodeproj
** BUILD FAILED **


The following build commands failed:
	CompileSwift normal arm64
	CompileSwiftSources normal arm64 com.apple.xcode.tools.swift.compiler
	CompileSwift normal armv7
(3 failures)
/Users/Jeremy/Downloads/Loop_Home/Loop/Carthage/Checkouts/LoopKit/CarbKit/CarbMath.swift:451:35:
 error: value of optional type 'TimeInterval?' (aka 'Optional<Double>') not unwrapped; did you mean to use '!' or '?'?
/Users/Jeremy/Downloads/Loop_Home/Loop/Carthage/Checkouts/LoopKit/CarbKit/CarbMath.swift:451:35:
 error: value of optional type 'TimeInterval?' (aka 'Optional<Double>') not unwrapped; did you mean to use '!' or '?'?
A shell task (/usr/bin/xcrun xcodebuild -project /Users/Jeremy/Downloads/Loop_Home/Loop/Carthage/Checkouts/LoopKit/LoopKit.xcodeproj -scheme CarbKit -configuration Release -sdk iphoneos ONLY_ACTIVE_ARCH=NO BITCODE_GENERATION_MODE=bitcode CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY= CARTHAGE=YES clean build) failed with exit code 65:
** BUILD FAILED **


The following build commands failed:
	CompileSwift normal arm64
	CompileSwiftSources normal arm64 com.apple.xcode.tools.swift.compiler
	CompileSwift normal armv7
(3 failures)

@jeremybarnum

@jeremybarnum unfortunately, I'm unable to even read swift properly, let alone code anything. We may not be able to do much on this issue without some assistance.

yeah that makes two of us, with the added disadvantage in my case that I can't code in any language!

@ps2

@ps2 help! :-)

Do you have the source code shared somewhere? Typically you have to write something like if let x = optional_input { } or guard let x = optional_input else { return } You can also force this with ! but that can lead to runtime crashes. Without knowing the code you are working on it is hard to help :) Line 451 of https://github.com/LoopKit/LoopKit/blob/master/CarbKit/CarbMath.swift is a comment.

…

On Fri, Sep 22, 2017 at 7:50 AM, francesc0-cgm ***@***.***> wrote: @ps2 <https://github.com/ps2> help! :-) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#577 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAE2aiK2X6WRs5IQgMWvD5X_D7SW4KEzks5sk0qtgaJpZM4PNcDC> .

-- Jan Dittmer <jdi@l4x.org>, http://l4x.org

@trixing

@trixing thanks for trying to help. I'm doing some more work today including figuring out how to do the remote repository thing. Not worth anyone's time yet.

@dm61

@dm61 @trixing I think I got this to work - now I'm having trouble reproducing the solution because my carthage update isn't working right after being perfect all night - not sure what's going on - probably a dumb late night mistake. But anyway I'm quite sure I got it working right and tested it out and saw the predictions and the carb decays working as I would have expected.

The initial idea above basically works. Two changes needed:

it's entry.absorptionTime not entry.AbsorptionTime

and it's entry.absorptionTime! to force unwrap it, as @trixing was saying. I guess that's not best practice but it seems to be OK in this case. I tried the nil case and didn't crash the app. I couldn't manage to do it properly with chained optionals or whatever the right way would be.

Here are the two lines in my remote repo. Fair warning that I tested linking to it and rebuilding Carthage dependencies and it didn't pick up the updates. Not sure why - probably the changes are not committed properly in the browser version or something. But when I did get it to work it was by doing the edits in xcode on my local repo and making sure I committed the changes and pointing the cartfile to the local repo.

https://github.com/jeremybarnum/LoopKit/blob/b14404520f83528c9611d8a1c687ba5f51a90dc1/CarbKit/CarbMath.swift#L449

https://github.com/jeremybarnum/LoopKit/blob/b14404520f83528c9611d8a1c687ba5f51a90dc1/CarbKit/CarbMath.swift#L504

@jeremybarnum

@jeremybarnum thanks for the notes on Carthage - I've managed to have my LoopKit updates successfully incorporated in Loop, so I am ready to test out your approach. Looks like you've replaced minimum absorption rate with expected absorption rate, which is fine, except this may be equivalent to just setting the absorptionTimeOverrun to 1.0, not exactly what were trying to do (not sure)?

Not exactly - which doesn't mean what I've done is necessarily right. I didn't delete minAbsorption rate, I added a new variable expectedAbsorption rate. I then only used that in one place, where estimatedTimeRemaining is calculated. Whereas (and this is where I would l love your confirmation) I think the "expiry" aka the minimum absorption rate is used elsewhere, mostly in the building of the carb entry objects. When I tested this last night I definitely saw the absorption time in the future consistent with absorptionOverrun being at 1.0 even though it's at 1.5 (ie when I put in 30 minute absorption, the prediction and the absorption screen detail were all using 30 minutes) while the expiring of the carbs (which was happening because BG was stable throughout) was happening at the minAbsorptionRate. Which was your original point/insight that I had been confused about - that you wan the expected rate and minimum rate to be different.

@jeremybarnum

@jeremybarnum yes, you're right, looks like minAbsorbtionRate still works as intended in the rest of dynamic carb code, although there may be some other side effects - will look into this some more. Great work, this might be it! I'll give it a whirl.

@jeremybarnum

@jeremybarnum I've been running the modified version for more than a week now. I think it works as intended, and I like it. Seems more natural to simply acknowledge user's CA input as is. No need to artificially enter shorter times just to force more aggressive dosing up front. Also confirmed that dynamic carb algorithm is still able to extend CA by 50% if need be. What are your impressions? Has anyone else tried this?

@dm61

Glad to hear you like it and it's working @dm61. We (meaning my wife) have been running this since Saturday with no issues. I don't have enough clean data to say definitely that it's better but anecdotally the temping behavior seems quieter (consistent with RC being anchored around true expectation). She is talking about switching to fiasp in the next couple of days where I think this change will be key. In any event I remain totally convinced that this is the more transparent and best approach.

I think one of the reasons people don't "notice" this issue that much is that with the longer insulin durations in 1.5.0 the under-bolusing for long CA meals is less common. And furthermore, the type of systematic, consistent "error" introduced by scaling up CA automatically is the sort of thing that RC is really good at dealing with. Plus, at the margin, this leads to a bit of early high temping that is offset later in the CA - which is sort of an accidental version of your alpha from #533 and might be aggressiveness that people like without being fully conscious of it.

The minimum absorption rate isn't just for allowing the carbs to continue showing absorption past the entered rate. It's also a hypo mitigation. Loop cannot "take back" insulin. I understand the desire to make the model match the user's input, and this would be fine, if we had a dosing amount computation that took into account uncertainty (larger meals would have more uncertainty about bg impact). But for now, the minimum absorption rate carries some of that load, and can't be changed in the manner described here.

LOOP-4664 Capture build details in separate plist (#577)

Navigation Menu

Repository navigation

Separate `absorptionTimeOverrun` into two variables - expectation and expiry #577

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Separate absorptionTimeOverrun into two variables - expectation and expiry #577

Description

Activity

dm61 commented on Sep 20, 2017

jeremybarnum commented on Sep 20, 2017

dm61 commented on Sep 20, 2017

jeremybarnum commented on Sep 22, 2017

dm61 commented on Sep 22, 2017

jeremybarnum commented on Sep 22, 2017

francesc0-cgm commented on Sep 22, 2017

trixing commented on Sep 22, 2017

jeremybarnum commented on Sep 22, 2017

jeremybarnum commented on Sep 23, 2017

dm61 commented on Sep 23, 2017

jeremybarnum commented on Sep 23, 2017

dm61 commented on Sep 23, 2017

dm61 commented on Oct 3, 2017

jeremybarnum commented on Oct 3, 2017

ps2 commented on Dec 7, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Separate `absorptionTimeOverrun` into two variables - expectation and expiry #577