Fix Stale Autotune Profile #1101
Conversation
|
@tim2000s, it won't affect overriding the Mandated minimum DIA since that mandate is enforced in the determine IOB calculation code regardless of the settings passed in. I didn't change how ISF is handled because autotune tunes ISF similar to basals. It looked like the code already respects the boundaries on how far ISF can be moved from the pump profile which would get updated if the pump setting changed. |
bin/oref0-autotune-prep.js
Outdated
| profile_data.curve = pumpprofile_data.curve; | ||
|
|
||
| // if not tuning the insulinEndTime and insulinPeakTime, then use data from pump profile | ||
| if (!tune_insulin_curve) { |
There was a problem hiding this comment.
This would change those values back to defaults on subsequent runs if you tune insulin curve manually and have autotune running on cron. Do we want that? Or should we stick with the tuned insulin curve?
There was a problem hiding this comment.
insulinPeakTime will track the user's value changes entered in preferences.
Perhaps we should add a preference for customDIA and dia to allow the user to manually override the value from the pump? That way, if they aren't intentionally overriding the pump value, it tracks the pump value if the user changes it.
There was a problem hiding this comment.
I suspect the more common use case is that someone will only have DIA set in the pump, and then will run tune_insulin_curve in a manual autotune run. I don't want to impose too many hoops: it seems like the default behavior should be "if you've tuned insulinPeakTime or insulinEndTime, keep using the tuned values"...
There was a problem hiding this comment.
That makes sense for that to be the more common use case.
Any thoughts on how to make the fact that curve values have been tuned persist so it could test for that case?
There was a problem hiding this comment.
Would it be as simple as checking whether useCustomPeakTime is set (whatever it's called exactly) instead of checking to see if we're doing tune_insulin_curve at the moment?
There was a problem hiding this comment.
Can I add a flag to the autotune output profile it would set to true if tune_insulin_curve had been run? Testing the flag instead of the argument would then be used to determine if it needs to keep the dia and peak from the previous autotune profile vs. pull it in from the pumpprofile.
There was a problem hiding this comment.
I’m proposing we just use useCustomPeakTime as that flag.
https://github.com/openaps/oref0/blob/master/lib/profile/index.js#L5
There was a problem hiding this comment.
Yep, I was overthinking it. Changed to use useCustomPeakTime.
|
Just a note that this has an interaction with #1114. Whichever is merged first, the other will need to be updated. |
jpcunningh
force-pushed
the
keep-autotune-insuline-curve-updated
branch
from
0bcc461 to
6f596b2
Compare
|
Rebased onto #1106 to resolve conflict. |
|
Looks like merging the other PRs introduced another merge conflict. |
Use pump profile dia and insulinPeakTime if not tuning insulin curve.
jpcunningh
force-pushed
the
keep-autotune-insuline-curve-updated
branch
from
fc193d5 to
c0714a2
Compare
|
rebased to dev to resolve merge conflict. |
Added command tests for new option in autotune-core.
scottleibrand
left a comment
scottleibrand
left a comment
There was a problem hiding this comment.
LGTM. Ready to merge?
|
Not yet. There is a question in the thread above about default behavior. |
dia and insulinPeakTime from previous autotune or to use the pump profile values.
|
Ready to merge. |
* Keep insulin curve, peak, and dia up to date. * Migrated oref0-autotune-core.js argument processing to yargs. * Removed unused option. * Added tune insulin curve argument to autotune-core * Add tune-insulin-curve option to oref0-autotune-core Use pump profile dia and insulinPeakTime if not tuning insulin curve. * Fixed arg processing in autotune-core for tune-insulin-curve. * Fixed error in autotune-prep introduced by the rebase. Added command tests for new option in autotune-core. * Use useCustomPeakTime to determine whether to persist dia and insulinPeakTime from previous autotune or to use the pump profile values.
3 participants
This is an attempt to address #1093.
Having tested Fiasp a couple of weeks ago, we noticed the same behavior.
This uses the pump profile to keep curve, dia, and peak up to date.