New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exponential curves #568

Merged
merged 34 commits into from Aug 13, 2017

Conversation

Projects
None yet
8 participants
@sulkaharo
Collaborator

sulkaharo commented Jul 25, 2017

Implements #544 - needs testing help, do not merge until confirmed to work.

NOTE: consider testing this extremely experimental! This code branch implements the new exponential curve model for insulin activity. This is a fairly big change compared to the old bilinear model, so if you do participate in the testing of this feature, assume you will need to monitor your BG closely and might need to adjust your profile to fit.

The code now supports three new configuration variables in preferences.json:

  • curve, with legal values of blank, "bilinear", "rapid-acting" and "ultra-rapid"
    • If left blank or set to "bilinear", OpenAPS uses the old insulin activity model
    • If set to "ultra-rapid", the new curve model analyzed from publicly available data for Fiasp from Novo Nordisk is used, set to reach activity peak at 55 minutes
    • If set to "rapid-acting", the new curve model analyzed from publicly available data for Novorapid (Novolog) from Novo Nordisk is used. This curve should match well to Novorapid, Novolog, Humalog and Apidra and is set to peak at 75 minutes
  • insulinPeakTime, with legal values from 35 to 120. The value defines the minutes from bolus to peak. Note if you don't set it, the above mentioned peak defaults are used.
  • useCustomPeakTime, which must be set to "true" for the customized insulinPeakTime value to take effect

In order to use the rapid-acting or ultra-rapid curves, your pump must be set to 5 hour DIA, or longer. 6 hours is recommended as the starting point.

The underlying mathematical model is developed by @dm61.

Below are the activity and IOB graphs at various DIA and peak values

act new

iob new

Testing

Both Fiasp and Novorapid curves are modeled after 5 hour DIA (compared to 3 hour default DIA for the old model). Due to the curve shape, the insulin activity contribution at the 5 hour mark is extremely low but should model the actual insulin tail more effectively than the old model, so it'd be extremely beneficial to get testing help from users who currently have their DIA set to a lower value (3 or 4 hours), who would set their DIA to 5 hours with the new curve model and report back if OpenAPS correctly keeps the basal rates in balance.

For Fiasp users, please start testing with DIA = 6, curve = "ultra-rapid" and do not set insulinPeakTime. If it looks like Fiasp is peaking for you faster or slower than OpenAPS expects (similar to your DIA being shorter or longer in the old parlance), please adjust insulinPeakTime by a small amount (such as 5 minutes) from the default of 55.

Note on Fiasp

Multiple users have reported cab take up to a month for the body to fully to adapt to Fiasp. If your body needs to adapt, you're likely to see the first few days post adopting Fiasp to work well and then hit a sudden resistance period, where your total daily dose will increase substantially and you might need to adjust every setting in your profile (ISF, carb ratios, basals). Cause for this is unknown but this is expected on some individuals and this doesn't indicate the loop is behaving unexpectedly.

@PieterGit

This comment has been minimized.

Contributor

PieterGit commented Jul 25, 2017

How can you find/calculate the insulinPeakTime that must be used?
Or should I first test Novorapid with insulinPeakTime 55 min and DIA=5h?

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 25, 2017

@PieterGit Start with DIA=5 and curve = fiasp. The peak time defaults to 55, which is the "official" peak from Novo.

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 25, 2017

Oops, @PieterGit the peak time setting is currently only supported for Fiasp. I'll look at adding it for Novo, too.

@mgranberryto

This has nothing to do with this PR per se, but I have noticed a recent performance regression that I noticed when I started using the exponential curves.

I had gone a while between loop updates prior to implementing the exponential curves so this isn't necessarily the cause, but oref0-meal has gone from returning almost instantaneously to requiring well over a minute to execute in my loop. I don't know whether it's because the IOB formula is now significantly more resource-intensive to calculate (non-integral powers are computationally expensive) or because oref0-meal is operating on significantly more data with the merged pump history or both, but it is leading to a non-trivial amount of extra load on my Edison and I can no longer loop through the night if I forget to plug in at night. SMB boluses are also coming in minutes later now, which should lead to more control instability. It's small, but small things add up.

I have thought about pre-calculating the iob/activity values for "1 unit" from 0 ... dia minutes and returning those cached values scaled by the insulin dose. It would also be possible to calculate the current insulin math as a cross-correlation of the IOB and activity arrays with a per-minute insulin delivery vector. In an experiment in Python I found that to result in near-instantaneous all-day calculations, but I haven't gone through the effort of implementing it in JavaScript.

if (treatment.insulin) {
var bolusTime = new Date(treatment.date);
var minAgo = diaratio * (time - bolusTime) / 1000 / 60;

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

If you have the spreadsheet up, I'd like to see how the peak time adjustments affect NovoRapid too. This looks to me like it will completely break as it currently stands.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

Changed the code so it uses 75 minutes as the baseline for Novo.

curve = curve.toLowerCase();
if (curve != 'novorapid' && curve != 'fiasp') {

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

I know I didn't include it in the code you started with, but it is probably worth including 'novolog' as a curve option too, as an alias for 'novorapid'.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

👍 Renamed the curves to generic names, will push soon.

return;
}
var usePeakTime = false;

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

It's a nitpick, but it looks like indentation is irregular here. It's probably worth cleaning this up or using an editor to reformat the function.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

Reformatted!

console.error('Pump DIA must be set to 5 hours if using the Insulin Peak Time setting');
if (profile.insulinPeakTime < 35 || profile.insulinPeakTime > 80) {
console.error('Insulin Peak Time is only supported for values between 35 to 80 minutes');

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

Perhaps this should come earlier and it should be restricted to fiasp only.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

Added scaling for Novo, too, and expanded the range.

var diaratio = 5.0 / profile.dia;
if (profile.insulinPeakTime !== undefined) {

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

The indentation is different here too.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

Reformatted.

if (minAgo < end) {
if (curve.toLowerCase() === 'fiasp') {
iobContrib = treatment.insulin * (((minAgo / 55) + 1) * Math.exp(-(minAgo) / 55));

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

Does the peak scaling change the AUC for activity? I haven't done the math, but it feels like it might. A normalization term might be needed.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

Yeah the AUC changes; here's what the AUC normalized curves look like. The change in the calculated activity is pretty dramatic, wonder if this would work.

auc normalized

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 25, 2017

My Excel that made the graphs

iob act model.xlsx

Force IOB to 0 if over 5 hours have passed. Added tests, support for …
…Novo peak scaling. AUC normalized for activity.
@mgranberryto

It's a little tough seeing what changed in the tests through Github's interface. Did you just add a couple tests?

This appears to address the concerns that I had, aside from verifying that the peak-scaled and normalized IOB and activity formulas correspond with eachother. Intuitively they look like they should, but I haven't yet done the math.

afterDIA.bolussnooze.should.equal(0);
});
it('should calculate IOB with Fiasp', function() {

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

Should this text be changed for consistency now that fiasp has been changed to ultrafast?

});
it('should calculate IOB with Fiasp peak setting of 55', function() {

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

It says "Fiasp" here too.

console.error('Unsupported curve function: "' + curve + '". Supported curves: bilinear, novorapid and fiasp.');
return;
if (curve != 'fast-acting' && curve != 'ultrafast') {

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

rapid-acting is the language commonly used in the USA for Novolog, Humalog, and Apidra, and short-acting for regular. Would rapid and ultra-rapid work better here?

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

👍 changed

iobContrib = treatment.insulin * (1 - 0.001852 * x * x + 0.001852 * x);
//activityContrib=sens*treatment.insulin*(2/dia/60/peak)*minAgo;
activityContrib = treatment.insulin * (2 / dia / 60 / peak) * minAgo;
} else if (minAgo < end) {
var y = (minAgo-peak)/5;
var y = (minAgo - peak) / 5;
iobContrib = treatment.insulin * (0.001323 * y * y - .054233 * y + .55556);
//activityContrib=sens*treatment.insulin*(2/dia/60-(minAgo-peak)*2/dia/60/(60*dia-peak));
activityContrib = treatment.insulin * (2 / dia / 60 - (minAgo - peak) * 2 / dia / 60 / (60 * 3 - peak));

This comment has been minimized.

@mgranberryto

mgranberryto Jul 25, 2017

I haven't tested this, but I want to verify that activity integrates to something ~= treatment.insulin post-scaling. Strange things will happen if it doesn't.

This comment has been minimized.

@sulkaharo

sulkaharo Jul 25, 2017

Collaborator

This is the old algorithm, which I'm leaving exactly as-is

@mgranberryto

This comment has been minimized.

mgranberryto commented Jul 25, 2017

Thanks for making the effort to push this across the finish line. This is going to make a huge difference.

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 25, 2017

@mgranberry thanks for the comments! Better get this right <3

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 25, 2017

Here's the current activity curve for rapid-acting model

rapid-acting

@ghost

This comment has been minimized.

ghost commented Jul 26, 2017

Have remaining 85U Humalog in my reservoir, with a new reservoir will give Fiasp a 2nd trial testing the curve adjustments

@PieterGit

This comment has been minimized.

Contributor

PieterGit commented Jul 26, 2017

testing this with jubi 0.2.0 rig on node 6.11.1 and with curve=rapid-acting (novorapid). First results are nice (quite flat). Please also add the curve preference to oref0/lib/profile/index.js. CPU usage of the rig seems a bit higher.

Small speedup to IOB calculation, use consistent date value if missin…
…g, added default curve to profile to ensure the correct curve is used
@ghost

This comment has been minimized.

ghost commented Jul 27, 2017

running now w/ Fiasp and ultra-rapid, today breakfast 7.00 am, 85g carbs, 11U Fiasp per pump, here the curve 4 hours later
fiasp-1
fiasp-2

@sulkaharo

This comment has been minimized.

Collaborator

sulkaharo commented Jul 27, 2017

If you're on this branch, please update to the latest push. I made a fix to the profile.json generation that ensures the new curves are actually being used at all times.

Revert "Merge branch 'cob-test' into 0.6.0-dev"
This reverts commit 72a60d9, reversing
changes made to 547376a.

Conflicts:
	lib/iob/total.js
@tim2000s

This comment has been minimized.

Contributor

tim2000s commented Aug 12, 2017

Given @sulkaharo 's investigations into the maths of the existing bilinear curves, is it worth leaving out the bilinear option and simply releasing 0.6.0 with the updated "rapid" and "ultra-rapid" curves? We know that these scale safely and produce good results, and it will remove the broken maths for bilinear in the process.

@tim2000s

This comment has been minimized.

Contributor

tim2000s commented Aug 12, 2017

I'm running exponential curves branch on a rig with node 0.10.29, updated to the latest versions of the code. Preferences are:

"max_iob": 12,
        "max_daily_safety_multiplier": 3,
        "current_basal_safety_multiplier": 4,
        "autosens_max": 1.2,
        "autosens_min": 0.7,
        "rewind_resets_autosens": true,
        "autosens_adjust_targets": true,
        "adv_target_adjustments": true,
        "maxCOB": 120,
        "override_high_target_with_low": false,
        "skip_neutral_temps": false,
        "unsuspend_if_no_temp": false,
        "bolussnooze_dia_divisor": 2,
        "min_5m_carbimpact": 8,
        "carbratio_adjustmentratio": 1,
        "autotune_isf_adjustmentFraction": 0.5,
        "remainingCarbsFraction": 1,
        "remainingCarbsCap": 90,
        "enableUAM": false,
        "enableSMB_with_bolus": false,
        "enableSMB_with_COB": false,
        "enableSMB_with_temptarget": false,
        "enableSMB_after_carbs": false,
        "curve": "bilinear",
        "useCustomPeakTime": false,
        "insulinPeakTime": 75

And the loop is running correctly. See logs:

img_4099

@scottleibrand scottleibrand merged commit 9683781 into 0.6.0-dev Aug 13, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.7%) to 36.167%
Details

@scottleibrand scottleibrand deleted the exponential_curves branch Aug 13, 2017

@tynbendad

This comment has been minimized.

Contributor

tynbendad commented Aug 13, 2017

does this change include a finer control over DIA (e.g., 4.5 hours vs 4 or 5)? or do you think i will no longer want that after using the new curves? right now 4 has trouble giving lows overnight and 5 doesn't quite get us down to target overnight, i'm hoping a longer autotune on 5 will fix that but not sure...

@tim2000s

This comment has been minimized.

Contributor

tim2000s commented Aug 13, 2017

@tynbendad With exponential curves, DIA is no longer used for calculating insulin IOB or action.

@tynbendad

This comment has been minimized.

Contributor

tynbendad commented Aug 13, 2017

thanks & great, that makes things simpler, i think...

@ghost

This comment has been minimized.

ghost commented Aug 14, 2017

Q to the curve set in preference.json, any information in the log which curve is set?

@scottleibrand

This comment has been minimized.

Contributor

scottleibrand commented Aug 14, 2017

Not currently. Perhaps we should have determine-basal print the curve-related preferences? Want to PR that?

@tim2000s

This comment has been minimized.

Contributor

tim2000s commented Aug 15, 2017

@sulkaharo mentioned he was planning to add that.

@tynbendad

This comment has been minimized.

Contributor

tynbendad commented Aug 17, 2017

just curious in case its needed by someone someday, is it possible to add a curve for regular insulin ("R")?

@scottleibrand

This comment has been minimized.

Contributor

scottleibrand commented Aug 17, 2017

Possible, sure. Worth it? Most definitely not, until/unless someone needs it.

@mgranberryto

This comment has been minimized.

mgranberryto commented Aug 17, 2017

To my eye a peak of 90 and dia of about 8h approximates R well enough, but there a lot of other assumptions being made in the system about insulin activity that make it a bigger task than it immediately appears to use "R." I suspect there aren't many (any?) users using OpenAPS with regular insulin.

@PieterGit

This comment has been minimized.

Contributor

PieterGit commented Aug 27, 2017

@scottleibrand : did you manage to resolve the stability issues you mentioned here #568 (comment) ?

I ran the curve = bilinear for about a week on my branch https://github.com/PieterGit/oref0/commits/201707_dev a while ago and did not have any stability problems. The branch was a bit to eager SMB'ing and sometimes caused hypo's.

Do people use 0.6.0 with this curve=rapid-acting on a daily basis now?

@scottleibrand

This comment has been minimized.

Contributor

scottleibrand commented Aug 27, 2017

I fixed the performance issues with some optimizations in 0.6.0-dev, but haven't retested Sulka's PR. We're using ultrarapid and it's working quite well.

@tim2000s

This comment has been minimized.

Contributor

tim2000s commented Aug 28, 2017

I've been running with ultra-rapid for weeks now with no stability issues.

@scottleibrand scottleibrand referenced this pull request Sep 6, 2017

Merged

Understand iob calcs #1033

@philipgo

This comment has been minimized.

Contributor

philipgo commented Sep 7, 2017

Should the line in preferences.json read "curve": "ultra-rapid" or "curve": ultra-rapid ?

From the code I would expect ultra-rapid without quotation marks but Tim`s example has quotation marks. What is the correct form?

@scottleibrand

This comment has been minimized.

Contributor

scottleibrand commented Sep 7, 2017

"curve": "ultra-rapid" is how it should appear in preferences.json.

@ghost

This comment has been minimized.

ghost commented Sep 13, 2017

short Q, for the new curves I need to do a checkout 0.6.0-dev?
a rough ETA for availability in dev (the curves)?

@scottleibrand

This comment has been minimized.

Contributor

scottleibrand commented Sep 13, 2017

Correct. We are just waiting for someone to have some time to do some documentation before we merge 0.6.0-dev to dev for broader testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment