nightscout / Trio

MIT License
45 stars 127 forks source link

Remove Profiles from Open-iAPS Oref and move to Swift #181

Open bjornoleh opened 1 month ago

bjornoleh commented 1 month ago

Copied from @dnzxy's comment in Discord (https://discord.com/channels/1020905149037813862/1226878954535649351/1228787378529894411)

the way this currently works is this:

The this should work is all swift based and the only things that need to be passed to oref would be the current basal rate, the current isf rate, the current cr rate and all those modified by the selected %.

If you set the profile to disable SMBs, that can also happen Swift-based

determine basal should ideally only do exactly that: determine required insulin, not have a logic for adjusting setting metrics or basal rates.

avouspierre commented 1 month ago

Before refactoring code, need to be agree about the algorithm : See #214

dnzxy commented 1 month ago

Before refactoring code, need to be agree about the algorithm : See #214

What do you mean by „agree on algorithm“? 😊 Having a common understanding of how it currently works or agreeing on necessary changes? 🤔

avouspierre commented 1 month ago

What do you mean by „agree on algorithm“? 😊 Having a common understanding of how it currently works or agreeing on necessary changes? 🤔

Have a common understanding of how it currently works... For example TDD is based on 2 last hours and not 24 previous hours...

Secondly, what's the purpose of this enhancement ? Come back to the initial oref0 code or only remove override ? iAPS 4 has chosen to use the "initial" oref0 and to push all code in a specific javascript (prepare.js) called before the determine basal function - Principe of a middleware or proxy -. Do we go in the same target with Trio ? Do you move only in a swift code before calling the "initial" oref0 ?

MikePlante1 commented 1 month ago

Have a common understanding of how it currently works... For example TDD is based on 2 last hours and not 24 previous hours...

I’ll have to go in and look closer at this later, but I thought the 2 hours value was the 24hr TDD from each loop cycle from the past two hours averaged together.

EDIT: I did confirm this looking at the code.

dnzxy commented 1 month ago

Awesome, thanks for clarifying 😊 What iAPS has done is what most of the people involved here have said needs to be done; they were just quicker with it now, much to the quality of how it‘s been done if I’m frank.

Whatever we end up doing, please not do what they have done and pull Javascript out of Javascript only to put Javascript wrapped in JS worker into Swift, and duplicate data by storing them both in CoreData entities and JSON files.

I’d still vote for looking to utilize plain oref0 and additional layers should either adjust input or output; it does not belong into determine basal 😊 ideally, we‘d use what Milos / AAPS has rewritten in Kotlin 👍

Edit to add: we should also look into streamlining dynamic ISF calculation and TDD calculation / averaging to how it‘s done in AAPS, and utilize the expertise of various people (Chris, Tim, Tom, Milos) when doing so. We should look at this as a unified approach to oref on iOS and Android - their knowledge should be our‘s to make use of 😊

bjornoleh commented 1 month ago

Have a common understanding of how it currently works... For example TDD is based on 2 last hours and not 24 previous hours...

I’ll have to go in and look closer at this later, but I thought the 2 hours value was the 24hr TDD from each loop cycle from the past two hours averaged together.

That’s correct. A new 24 hour TDD value is calculated every five minutes. To reduce the impact of timing of meals (and associated meal bolus), the 24 TDD values over the last two hours is averaged. They still represent an estimate for a 24 hour TDD, but a smoother one than the raw five minute values.

github-actions[bot] commented 13 hours ago

hey 👋 - silence for 30 days 🤐 ... anybody? triage is required!