Skip to content
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

halfBasalExerciseTarget and lowTT settings can lead to negative ISF and Basal #1401

Open
jncchds opened this issue Jul 6, 2021 · 10 comments

Comments

@jncchds
Copy link
Contributor

jncchds commented Jul 6, 2021

When we set halfBasalExerciseTarget to (for example) 110 and lowTempTarget of 80, the sensitivityRatio is negative
halfBasal of 100 leads to sensitivity of 0
the code with the calculations is in determine-basal.js:
https://github.com/openaps/oref0/blob/master/lib/determine-basal/determine-basal.js#L209

formula is (halfBasalTarget - 100) / (halfBasalTarget - 100 + target - 100)
in the situation mentioned above we get (110 - 100) / (110 - 100 + 80 - 100) = 10 / (-10) = -1
that leads to negative basal and ISF and calculations are all wrong

setting halfBasal to 100 leads to ratio = 0.0 and thus when we calculate ISF, we divide the original ISF by the ratio, so divide by zero

this can happen only on low TT because the numerator is almost guaranteed to be above 0 and with the denominator of
halfBasalTarget + target - 200 we get negative results only with low targets
and low TT usually require low sensitivity, so we may set it as a max available value in preferences

I suggest we add the check after the sensitivityRatio = Math.min(sensitivityRatio, profile.autosens_max); as follows:
if (sensitivityRatio <= 0.0) sensitivityRatio = profile.autosens_max;

To Reproduce
Steps to reproduce the behavior:

  1. set halfBasalExerciseTarget to 110
  2. set low temptarget lowers sensitivity to true
  3. activate TT of 80
  4. see the negative ratio, ISF and Basal needed

the second issue:

  1. set halfBasalExerciseTarget to 100
  2. set low tt lowers sensitivity / high tt raises sensitivity
  3. activate low / high tt
  4. see the determine-basal fail

Expected behavior
With low TT the ratio needs to be high

Smartphone (please complete the following information):

Setup Information (please complete the following information):

  • All pump, CGMs and rigs are affected
  • the issue exists in the current master branch
@mountrcg
Copy link
Contributor

mountrcg commented Jul 6, 2021

There are several possibilities to arrive at denominator of 0 in marked formula.
eg:
set halfBasalExerciseTarget to 120
set low tt lowers sensitivity
activate low tt of 80
.. and so on, always symmetric around 100.

@SeregaYakovlev
Copy link

SeregaYakovlev commented Jul 6, 2021

I confirm these bugs. My screenshots.
Negative InsulinReq, negative senstitivity ratio, negative ISF with target 81 and half_basal_exercise_target 108:
photo5413536385614328682

Deviding to zero with half_basal_exercise_target 100:
photo5413536385614328749
My preferences.json with half_exercise_basal_target: 100:
photo5413738596969592923

@jncchds
Copy link
Contributor Author

jncchds commented Jul 6, 2021

Following the comment of OpenAPS not allowing TTs lower than 80, updated the example numbers

@scottleibrand
Copy link
Contributor

I've confirmed the behavior described. The half_basal_exercise_target was never intended to be set that low, nor tested at such low values. I would prefer that we set a hard-coded minimum value for half_basal_exercise_target. Since the minimum temp target is hard-coded to 80 mg/dL, and the normalTarget is hard-coded to 100 mg/dL, a minimum value of >120 mg/dL for half_basal_exercise_target is required. If it's set lower than that, should we automatically bump it up to 130 or 125 mg/dL?

@jncchds
Copy link
Contributor Author

jncchds commented Jul 7, 2021

@scottleibrand I'm using halfBasal of 110 and TT of 144 to get sensitivity ratio down to ~20%
if I were to use 130, I would have to set TT to 220 for the same effect which is too high

@jncchds
Copy link
Contributor Author

jncchds commented Jul 7, 2021

alternatively we can do
if (c * (c + target_bg-normalTarget) <= 0.0) sensitivityRatio = profile.autosens_max; which will guarantee that we don't devide by zero or anything like that

@scottleibrand
Copy link
Contributor

Feel free to submit an alternative PR if you have specific code tweaks you think would work better.

jncchds added a commit to jncchds/oref0 that referenced this issue Jul 8, 2021
getting multiplication less or equal to 0 means that we have a really low target with a really low halfBasalTarget
with low TT and lowTTlowersSensitivity we need autosens_max as a value
we use multiplication instead of the division to avoid "division by zero error"
@jncchds
Copy link
Contributor Author

jncchds commented Jul 8, 2021

I've added my PR as I'm using a low halfBasal to be able to get to even lower ratio than 50% without using a really high TT when workout

@SeregaYakovlev
Copy link

SeregaYakovlev commented Nov 21, 2021

@scottleibrand, hello!
I just tested that bug with negative ISF and denominator of 0.

result of halfBasalTarget 100 with normalTarget 100 in a browser console:
image

result of halfBasalTarget 110 with normalTarget 100 in a browser console:
image

We can see, that sensitivityRatio is 1.2.

My prefrences.json on openaps:
image

The result with target_bg 80, halfBasalTarget 100 on openaps log:
image

I can not see dividing by zero.
Openaps continues to work.
You can visit my repository: https://github.com/SeregaYakovlev/oref0/tree/dev

@scottleibrand
Copy link
Contributor

Is #1403 your preferred solution, or is there another change you'd rather make?

scottleibrand pushed a commit that referenced this issue Dec 18, 2021
getting multiplication less or equal to 0 means that we have a really low target with a really low halfBasalTarget
with low TT and lowTTlowersSensitivity we need autosens_max as a value
we use multiplication instead of the division to avoid "division by zero error"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants