search

nightscout / Trio

MIT License
64 stars 259 forks source link

Add separate setting for sigmoid adjustment factor #132

Closed MikePlante1 closed 3 months ago

MikePlante1 commented 3 months ago

Addresses issue https://github.com/nightscout/Open-iAPS/issues/106 by using separate variables for Adjustment Factor when using Logarithmic DynamicISF vs Sigmoid DynamicISF. Defaults AF to 0.8 for Logarithmic and 0.5 for Sigmoid.

This adds a new setting for Adjustment Factor so Logarithmic DynamicISF can default to 0.8 and Sigmoid DynamicISF can default to 0.5. This prevents new users from starting DynamicISF in Logarithmic mode with a default of 0.5 which seems to often lead to prolonged highs for many people. Also helpful for those who switch between Logarithmic and Sigmoid depending on their situation.

Screenshot 2024-04-14 at 10 10 05 AM
JeremyStorring commented 3 months ago

Just want a sanity check - changes in open-iaps-oref from here will make their way into open-iaps-oref?

JeremyStorring commented 3 months ago

Everything here looks good and makes sense :) Good work!

bjornoleh commented 3 months ago

Just want a sanity check - changes in open-iaps-oref from here will make their way into open-iaps-oref?

Yes, PR 21 in Oiref does this: https://github.com/nightscout/open-iaps-oref/pull/21

I added you as reviewer :-)

avouspierre commented 3 months ago

Tested on simulator. Seems Ok even no information is displayed on the "loop popup" about using sigmoid function.

bjornoleh commented 3 months ago

Tested on simulator. Seems Ok even no information is displayed on the "loop popup" about using sigmoid function.

Do we want to adress this?

MikePlante1 commented 3 months ago

Here’s my assumption of what’s happening there… I think Sigmoid requires 21+ hours of TDD data in order to be used and if Sigmoid is being used then it won’t show up in the Loop toggle. I’m guessing you tested as a brand new installation in a simulator?

I’ll give it a look in a sim in a bit to confirm, though. Unless my assumption is wrong?

avouspierre commented 3 months ago

Here’s my assumption of what’s happening there… I think Sigmoid requires 21+ hours of TDD data in order to be used and if Sigmoid is being used then it won’t show up in the Loop toggle. I’m guessing you tested as a brand new installation in a simulator?

I’ll give it a look in a sim in a bit to confirm, though. Unless my assumption is wrong?

Yes. I tested on a fresh install with less 24h of data. I though there is a info in loop toggle in iAPS but not sure

marionbarker commented 3 months ago

Please evaluate if PR needs to be updated or if there is an issue with dev.

Test Case: I downloaded the patch and tried to apply to dev (96dd9178, Merge pull request 143) and it does not apply:

 % git apply /Users/marion/Downloads/oi-pr-132.patch                      
error: patch failed: FreeAPS/Resources/javascript/bundle/determine-basal.js:1
error: FreeAPS/Resources/javascript/bundle/determine-basal.js: patch does not apply
error: patch failed: oref0_source_version.txt:1
error: oref0_source_version.txt: patch does not apply

How does this PR differ from PR#114 which does apply without errors.

MikePlante1 commented 3 months ago

How does this PR differ from PR#114 which does apply without errors.

Soo… PR#114 didn’t actually include the minimized oref changes, which is what’s causing the merge conflicts here. I think I’m going to close this PR and just update 114 instead by merging in dev and applying the oref updates.

MikePlante1 commented 3 months ago

Closed since alpha was merged into dev. See https://github.com/nightscout/Open-iAPS/pull/114 for future progress on this PR.