BgReading.find_new_curve, division by zero ?

[JohanDegraeve]

Hi,

while porting the Android code to actionscript, I'm getting a division by zero error.

on this place :
https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/blob/master/app/src/main/java/com/eveningoutpost/dexdrip/Models/BgReading.java#L691
find_new_curve gets called during initial calibration. if there's only two readings stored.
This method gets called for each reading. Which means x2 will be the timestamp of the reading for which the method is called, x1 will in one the cases be the timestamp of the reading read from the database, which has exactly the same value,
which results in x1 - x2 being 0, and a division by zero error.

That's what happens in my code :
https://github.com/JohanDegraeve/iosxdripreader/blob/master/src/databaseclasses/BgReading.as#L513

in actionscript this gives "-Infinity" and the code just continues, but I would assume in Java this would throw an exception ?
(I could debug on android studio but this thing is so slow - if no one knows the answer I'll have to do it anyway)

thanks

Johan

[AdrianLxM]

I don't see why x1 and x2 are the same. They should be 5 minutes apart.

[AdrianLxM]

Both values are read from the DB: https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/blob/master/app/src/main/java/com/eveningoutpost/dexdrip/Models/BgReading.java#L658

This means they really should be 5 minutes apart. But as both calls query the same db with the same query the result for their curve defined by a,b and c should be the same.

[tzachi-dar]

I second adrian,

On xDrip there is code that checks that there is at least two minutes
difference between two packets.
This code is in:
TransmitterData create().

So for xDrip there should be no issue.

On Wed, Jul 27, 2016 at 2:20 AM, AdrianLxM [email protected] wrote:

Both values are read from the DB:
https://github.com/StephenBlackWasAlreadyTaken/xDrip-Experimental/blob/master/app/src/main/java/com/eveningoutpost/dexdrip/Models/BgReading.java#L658

This means they really should be 5 minutes apart. But as both calls query
the same db with the same query the result for their curve defined by a,b
and c should be the same.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#365 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHkw5NqplyPsFQOHD4glfCeMx0DGHD27ks5qZpY2gaJpZM4JVnTp
.

[JohanDegraeve]

Ok i'll have to start debugging both versions and compare the results step by step, I'm not saving the bgreadings to database at the same moments as the android app - could be the reason
I'll keep you informed

[JohanDegraeve]

was an error in my code

[JohanDegraeve]

sorry, it does seem to occur in the android version.
I have a branch here which adds traces and allows me to send them via e-mail (homeview, top right menu has an item "E'-mail logfile"
This is the result.
xdrip-log-2016-08-04-22-45-19.txt

What I do : install the app, start the sensor (shift back the time with exactly two hours) and wait for minimum two readings, when app says ("calibrate") then I do the double calibration.
In BgReading.find_new_curve, the value of second_latest is being printed. On line 219 of the logfile it shows "Infinity"
Actually this value has been set in the previous call to find_new_curve.
find_new_curve is being called two times in Calibration.initialCalibration. Once for the first calibration, once for the second calibration.

I think actually the error is here : https://github.com/JohanDegraeve/xDrip-Experimental/blob/master/app/src/main/java/com/eveningoutpost/dexdrip/Models/BgReading.java#L684

it says "double x2 = timestamp;"
it should be "double x2 = latest.timestamp;"

[tzachi-dar]

It seems that you are right, in your analysis of the problem. The function is called on the two points.

I wonder what will happen once we fix this. It seems to me that the b part only influences the predictive code, which is off by default.

[JohanDegraeve]

I don't understand the algorithms itself - but I think this is a rare case,
it only happens when the user shift the start time two hours back, and then
calibates immediately after having received two readings.
It occurred to me know because I was waiting for the readings to do the
tests.

2016-08-05 0:14 GMT+02:00 tzachi-dar [email protected]:

It seems that you are right, in your analysis of the problem. The function
is called on the two points.

I wonder what will happen once we fix this. It seems to me that the b part
only influences the predictive code, which is off by default.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#365 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANMwTfwDUiJjCsd5m3GoS1drbWm9CKM4ks5qcmRdgaJpZM4JVnTp
.

[tzachi-dar]

I think that your proposed solution is correct, but I want to think about it some more (and try it) before we move on.

[AdrianLxM]

If I understand this right it is a problem for the first of the two readings, not for the second as for the second latest.timestamp is the same as timestamp. It only occurs if there are just 2 readings.

As far as I remember the curve is used for future prediction. Either to directly show it on the screen or for an arrow symbol. It is always taken from the latest bg reading.

As it is calculated correctly for the second reading and never used for the first it should not be a problem in practice at all. It is still a bug in the code an we should correct it.

[JohanDegraeve]

Indeed only in case there's two readings, and only for one of the two readings it goes wrong.
It's already good to know for me how to correct this, because in my case (developping in ActionScript) I was getting an exception when trying to store this reading in the database. The rest of the code was not executed anymore. It's strange that there's no exception in the Android version. (although I didn't check exactly what happens in the debugger)

[AdrianLxM]

Only when there is 2 readings, only for one and only for the one we later never look at again ;)

A division by zero will only lead to an ArithmeticException if it is of int type. The floading point types also can evaluate to infinity or NaN.

[tzachi-dar]

I guess we all agree. I'll make a pr, will test it and commit it once we have some more confidence.

[JohanDegraeve]

ok

2016-08-06 23:25 GMT+02:00 tzachi-dar [email protected]:

I guess we all agree. I'll make a pr, will test it and commit it once we
have some more confidence.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#365 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANMwTfeX5QacWEY7xFY7deABf53aOov_ks5qdPuvgaJpZM4JVnTp
.

[tzachi-dar]

By the way, I think we have a similar bug also for the three packets case.
I'm fixing it as well, in my pr, I guess that for the same reasons AdrianLxM mentioned, it shouldn't affect anything.

[tzachi-dar]

Opened PR: #373

[AdrianLxM]

@tzachi-dar, good morning.
It seems like you have modified find_new_raw_curve but we've talked about find_new_curve here?
So we have this bug in both.

find_new_raw_curve directly affects calibrations output.
I've checked that save() is called everytime before find_new_raw_curve also -> the modification therefore should not do any changes in behaviour either.

[tzachi-dar]

Thanks for finding that, i'll fix both functions but it will take a few
days untill I can do it.

בתאריך 7 באוג׳ 2016 10:59 לפנה״צ,‏ "AdrianLxM" [email protected]
כתב:

@tzachi-dar https://github.com/tzachi-dar, good morning.
It seems like you have modified find_new_raw_curve but we've talked about
find_new_curve here?
So we have this bug in both.

find_new_raw_curve directly affects calibrations output.
I've checked that save() is called everytime before find_new_raw_curve
also -> the modification therefore should not do any changes in behaviour
either.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#365 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHkw5H9bvYpVH98fyy50py8N8OPvFueNks5qdZBGgaJpZM4JVnTp
.

[tzachi-dar]

OK, fixed the other function as well. Will test that and see if there are any problems with that.