jxx123 / simglucose

A Type-1 Diabetes simulator implemented in Python for Reinforcement Learning purpose
MIT License
232 stars 109 forks source link

Update risk.py #67

Open drozzy opened 10 months ago

drozzy commented 10 months ago

Fixing #63

drozzy commented 10 months ago

Do you know where in UVa/Padova Simulator they define it? I would be curios to see their implementation.

Regarding current use, I can't imagine people have used it because it produces wrong output at extreme values.

For example, here is the current risk if I let the glucose fall indefinitely: original_extreme_values

As you can see the function just becomes undefined at some point and gets set to zero.

Here is the proposed risk function for the same trajectory: proposed_extreme_values

Regarding compatibility, the new risk behaves the same for values in the acceptable range. Here is the current risk: original_risk

and here is proposed risk: proposed_risk

For large glucose values, the current risk is not a big problem as it does increase in the desired direction, however it is not bounded and goes to infinity: large_gluc_original_risk

the proposed risk caps this at 100: large_gluc_proposed_risk

I welcome more testing and feedback of my proposal, but believe it is especially important for automated optimizers, where such a drop may lead to incorrect learned behavior.

jxx123 commented 10 months ago

Do you know where in UVa/Padova Simulator they define it? I would be curios to see their implementation.

Regarding current use, I can't imagine people have used it because it produces wrong output at extreme values.

For example, here is the current risk if I let the glucose fall indefinitely: original_extreme_values

As you can see the function just becomes undefined at some point and gets set to zero.

Here is the proposed risk function for the same trajectory: proposed_extreme_values

Regarding compatibility, the new risk behaves the same for values in the acceptable range. Here is the current risk: original_risk

and here is proposed risk: proposed_risk

For large glucose values, the current risk is not a big problem as it does increase in the desired direction, however it is not bounded and goes to infinity: large_gluc_original_risk

the proposed risk caps this at 100: large_gluc_proposed_risk

I welcome more testing and feedback of my proposal, but believe it is especially important for automated optimizers, where such a drop may lead to incorrect learned behavior.

Wow, thank you so much for the detailed and compelling analysis. Yes, the current risk computation is definitely problematic.

FYI, here is how the original UVa/Padova Simulator implements the risk index (they sum up LBGI and HBGI), where i means i-th patient. Screenshot by Dropbox Capture

I think I will approve your change anyway since the existing one is definitely wrong. Thank you so much!

drozzy commented 10 months ago

Out of curiosity, where did you find this code? Is it accessible somewhere?

jxx123 commented 9 months ago

Out of curiosity, where did you find this code? Is it accessible somewhere?

I had it as in the 2008 UVa/Padova Simulator Academic version. You might be able to request a version from epsilon group https://tegvirginia.com/software/t1dms-2014/, but I heard stories that there was no reply from them.