Unit test improvements, small bug fixes

[sulkaharo]

• unit test for COB calculation - note the math is unverified
• New logic for ISF fetching
• Bug fix COB calculation if only one event is presented
• Sort glucose data to match later logic assumptions

[PieterGit]

@sulkaharo : is this branch ready for testing with node 8.1.x?
Can this maybe fix my issue: #587 ?

[scottleibrand]

If no objections, I'd like to merge this to dev for testing for a 0.5.3 release before we start merging new features (like exponential curves) for an eventual 0.6.0 release.

[PieterGit]

This PR made the travis-ci build fail for nodejs 0.10 and 0.12. But the travis-ci now succeeds for nodejs 8.1.4. I think a nodejs upgrade should be postponed to oref0 0.6.x and not be required for oref 0.5.3.

I have no objections of merging this branch to dev, but we should fix the travis-ci issues soon.
I will try to fix the main COB issue I'm having this weekend or beginning next week, see #587 .

[scottleibrand]

Agreed that the node upgrade, nogit, and exponential-carbs can all wait for 0.6.0.

[sulkaharo]

The fail is due to the COB unit test giving a random result on the server due to an unknown reason, which needs investigation. 0.6.0 sounds right

[scottleibrand]

Would it be possible/useful to merge these unit tests for 0.5.3 without the 8.1.4 requirement?

[scottleibrand]

Merged this to 0.6.0-dev. If there are any tweaks we can make to this to make it suitable for merging to dev for the eventual 0.5.3 release, I'd like to do that as well.

[scottleibrand]

Running #568 (with this branch merged in) on a rig with default preferences (bilinear curve) to start with. The pump-loop is never completing before getting killed off. When I run it manually, it's getting stuck on some of the new code from this branch.

root@edison-eb4 ~/myopenaps # service cron stop; killall -g oref0-pump-loop; openaps monitor-pump; service cron start
pump://JSON/read_clock/monitor/clock.json
reporting monitor/clock.json
pump://JSON/read_temp_basal/monitor/temp_basal.json
reporting monitor/temp_basal.json
pump://JSON/iter_pump_hours/monitor/pumphistory.json
reporting monitor/pumphistory.json
tz://JSON/rezone/monitor/pumphistory-zoned.json
reporting monitor/pumphistory-zoned.json
tz://JSON/clock/monitor/clock-zoned.json
reporting monitor/clock-zoned.json
iob://text/shell/monitor/iob.json
reporting monitor/iob.json
meal://text/shell/monitor/meal.json
Building ISF cache with 30 minute resolution
Found carbs: 2017-08-12T04:32:51.018Z , carbs= 60 , after absorption: 60

top shows oref0-meal spinning at 100% of both CPUs.

root@edison-eb4 ~/src # ps aux | grep 17341
root     17341 97.9  4.9 106796 48500 ?        Rl   22:03   4:40 node /usr/bin/oref0-meal monitor/pumphistory-merged.json settings/profile.json monitor/clock-zoned.json monitor/glucose.json settings/basal_profile.json monitor/carbhistory.json
root@edison-eb4 ~/src # ps aux | grep 17313
root     17313 98.1  4.6 104744 45784 pts/0    Rl+  22:03   5:21 node /usr/bin/oref0-meal monitor/pumphistory-merged.json settings/profile.json monitor/clock-zoned.json monitor/glucose.json settings/basal_profile.json monitor/carbhistory.json

After letting it run for more than 10 minutes, it finally finished:

Building ISF cache with 30 minute resolution
Found carbs: 2017-08-12T04:32:51.018Z , carbs= 60 , after absorption: 60
Found carbs: 2017-08-12T00:54:51.273Z , carbs= 90 , after absorption: 60
Found carbs: 2017-08-12T00:34:59.427Z , carbs= 150 , after absorption: 60
reporting monitor/meal.json
pump://JSON/reservoir/monitor/reservoir.json
reporting monitor/reservoir.json
pump://JSON/read_battery_status/monitor/battery.json
reporting monitor/battery.json
pump://JSON/status/monitor/status.json
reporting monitor/status.json

This particular rig is running node v6.11.1

[scottleibrand]

I re-did the 0.6.0-dev branch to not include this code, and retargeted the PR there to merge later once we figure out the performance issue.

[scottleibrand]

@sulkaharo We'll need to figure out how much of this we can incorporate into 0.6.0 (now in the dev branch) without negatively affecting performance. Any thoughts there?

[PieterGit]

@sulkaharo @scottleibrand : What's the best way to proceed with these COB unit tests? Will we skip the node upgrade for oref0 0.6.x release, or will make sure oref0 works with node6 or node8 LTS?

I would like to work on a unit test for #777 but, first would like to know how we proceed on this issue.

[scottleibrand]

Unless one of you have cycles to work on it, I plan to defer the node 8 upgrade until 0.6.1. 0.6.0 should work with node 8, but won't install it.

[sulkaharo]

I fixed the conficts with the code and merged to latest dev. Not sure where the performance issues are, when run locally this is the same performance as before. Note the test fails now though, the new COB math in 0.6.0 doesn't decay any of the carbs when the code in this test is run.

[sulkaharo]

The two places that can affect performance is the new ISF code - I just checked and reverting to 0.6.0 version didn't change the local runtime BUT noticed ISF test broke - looks like the 0.6.0 version miscalculates the ISF with profiles that have multiple ISF values. The performance is locally the same for both.

Another place that affects performance is the sorting of glucose data in meal/total.js which can be removed if we can guarantee it's always invoked with pre-sorted data - I added the sorting due to finding out the calculation breaks if there's a problem with the sorting.

[scottleibrand]

I fixed your sorting function: it was using timestamp (which doesn't exist for glucose) instead of date or dateString.

Now the tests are failing with Error: Cannot find module 'oref0/lib/determine-basal/cob' and similar. I changed one of them, which made it happy about that one, but there are a dozen other absolute require paths it's failing on now, so that doesn't seem like the real solution. Any idea why travis is failing on those here, but is fine in dev?

[scottleibrand]

Both make and make travis succeed locally for me...

[scottleibrand]

Is any of this still needed in 0.7.0?

[PieterGit]

@scottleibrand I think it would be good to have these fixes and more unit tests enabled for 0.7.0.
@sulkaharo Can you give an update on your thoughts with this PR?

[scottleibrand]

Closing due to lack of interest. Please re-open if you make any progress on fixing the tests.