Skip to content

Commit

Permalink
History database FIXED!!! (older data now works)
Browse files Browse the repository at this point in the history 
The problem was the distinction between:

"Last minute of the day"
      compared to
"how many minute are in a day"

These are not the same concept.

Last minute starts 60 seconds before midnight
They are numbered 0 to 1439, total is still 1440

This bug is related to an "off by one" error
I was really glad to see it was so easy to fix :)
  • Loading branch information
Sarah White committed Jul 5, 2014
1 parent d61cc6b commit 4ef18a9
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions core/candleManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ var equals = util.equals;

// even though we have leap seconds, every
// day has the same amount of minutes
var MINUTES_IN_DAY = 1439;

// these are NOT the same thing!!!
// -- kuzetsa, 2014 July 5th
var MINUTES_IN_DAY = 1440;
var START_OF_LAST_MINUTE_IN_DAY = 1439;


var Manager = function() {
_.bindAll(this);
Expand Down Expand Up @@ -343,8 +348,10 @@ Manager.prototype.checkDaysMeta = function(err, results) {
return false;
}

if(!isFirstDay && day.endCandle.s !== MINUTES_IN_DAY) {
// this day doesn't end at midnight
if(!isFirstDay && day.endCandle.s !== START_OF_LAST_MINUTE_IN_DAY) {
// the day REALLY DOES end at midnight, however...
// the last minute is 1 minute before midnight
// kuzetsa, 2014 July 5th
return false;
}

Expand Down Expand Up @@ -463,7 +470,7 @@ Manager.prototype.checkHistoryAge = function(data) {
return;
}

if(history.available.last.minute === MINUTES_IN_DAY) {
if(history.available.last.minute === START_OF_LAST_MINUTE_IN_DAY) {
this.increaseDay();
}

Expand Down Expand Up @@ -555,7 +562,7 @@ Manager.prototype.broadcastHistory = function(next, args) {
// get the candles for each required day
var iterator = function(mom, next) {
var from = 0;
var to = MINUTES_IN_DAY;
var to = START_OF_LAST_MINUTE_IN_DAY;

if(equals(mom.day, last.day)) {
// on first (most recent) day
Expand Down Expand Up @@ -676,7 +683,7 @@ Manager.prototype.processTrades = function(data) {
log.debug('fetch spans midnight');

// old day
batches[0] = this.addEmtpyCandles(_.first(batches), this.mostRecentCandle, MINUTES_IN_DAY);
batches[0] = this.addEmtpyCandles(_.first(batches), this.mostRecentCandle, START_OF_LAST_MINUTE_IN_DAY);

// new day

Expand All @@ -695,7 +702,7 @@ Manager.prototype.processTrades = function(data) {
// we already know they are all from current day

// but if the most recent candle is from yesterday ...
if(this.mostRecentCandle && this.mostRecentCandle.s === MINUTES_IN_DAY) {
if(this.mostRecentCandle && this.mostRecentCandle.s === START_OF_LAST_MINUTE_IN_DAY) {
ghostCandle = _.clone(this.mostRecentCandle);
ghostCandle.s = -1;
batch = this.addEmtpyCandles(candles, ghostCandle);
Expand Down Expand Up @@ -828,7 +835,7 @@ Manager.prototype.splitCandleDays = function(candles) {
// add empty candles. End is a minute # since midnight.
//
// for example:
// addEmtpyCandles(candles, 0, MINUTES_IN_DAY)
// addEmtpyCandles(candles, 0, START_OF_LAST_MINUTE_IN_DAY)
// would return an array of candles from:
// [midnight up to][batch without gaps][up to next midnight - 1]
Manager.prototype.addEmtpyCandles = function(candles, start, end) {
Expand Down

1 comment on commit 4ef18a9

@kuzetsa
Copy link
Owner

@kuzetsa kuzetsa commented on 4ef18a9 Jul 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's at least one spot which needs the accurate value of 1440 minutes per day, but erroneously used 1439 instead.
The ambiguity between   MINUTES_IN_DAY   [versus]   START_OF_LAST_MINUTE_IN_DAY   has been resolved.

    day.full = day.minutes === MINUTES_IN_DAY;

^ This code checks to make sure a historical data file had the entire 1440 minutes (24 hours) worth of data, but was getting confused because it had more than 1439 minutes, so returned a false negative thinking that "1440 minutes is too many"

Please sign in to comment.