jxx123 / simglucose

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

Controller is not reset during simulation of patients #43

Closed hannesvoss closed 2 years ago

hannesvoss commented 2 years ago

I discovered that in a batch simulation, there happens no controller reset between patient simulations, even if the controller reset() method is specified. I think this is a pretty serious flaw because in most cases only the first simulated patient will give correct results. I will prepare a PR for that issue, but wanted to hear your opinion on this issue first.

jxx123 commented 2 years ago

Yes, that is a real bug.

Need to reset it here https://github.com/jxx123/simglucose/blob/9dfa3c4648559b5ec1c25f32bcea968a3788527e/simglucose/simulation/env.py#L136.

Thanks for the report.

Let me know if you are interested in a PR.

hannesvoss commented 2 years ago

I'm interested and will open a PR soon.

jminlee97 commented 2 years ago

Hi, could you explain a bit more about what this issue is? I've been using the batch simulation regularly, and would be interested in knowing if this affects any of my experiments. What happens when the controller does not reset?

hannesvoss commented 2 years ago

In my case I trained a neural network that used a continuous chain of states and stored the internal state of the neural network (LSTM; hidden state). Due to the absence of the reset call, the hidden state was carried over from simulation to simulation and when a new simulation was started, a chain of states from the previous simulation + new states from the new simulation was used. This has led to a significant distortion of the results from the second simulation of a simulation batch onwards. Depending on your experiment setup this could lead to distorted results too.

hannesvoss commented 2 years ago

This issue is going to be solved when this PR is merged #47 .