Add linear regression functions #1063
Conversation
| } | ||
|
|
||
| case DerivType: | ||
| if len(args) != 1 { |
There was a problem hiding this comment.
you forgot to assign duration here. Bug ?
There was a problem hiding this comment.
All good- duration is not needed for deriv
Codecov Report
@@ Coverage Diff @@
## master #1063 +/- ##
==========================================
- Coverage 77.4% 77.39% -0.02%
==========================================
Files 545 546 +1
Lines 46068 46132 +64
==========================================
+ Hits 35659 35702 +43
- Misses 8156 8168 +12
- Partials 2253 2262 +9
Continue to review full report at Codecov.
|
src/query/functions/temporal/base.go
Outdated
| @@ -299,6 +299,7 @@ func (c *baseNode) processSingleRequest(request processRequest) error { | |||
| values = append(values, val) | |||
| newVal := math.NaN() | |||
| alignedTime, _ := bounds.TimeForIndex(i) | |||
| // fmt.Println("align: ", alignedTime.String()) | |||
| return emptyOp, fmt.Errorf("invalid number of args for %s: %d", PredictLinearType, len(args)) | ||
| } | ||
|
|
||
| duration, ok = args[1].(float64) |
There was a problem hiding this comment.
The duration for the actual query.
|
|
||
| var slope, intercept float64 | ||
|
|
||
| if l.op.operatorType == PredictLinearType { |
There was a problem hiding this comment.
This reads a little weird to me, since you're splitting processing amongst two functions and doing related calculations in both.
Instead, can you pull the code from linearRegression() into this Process function, and let it do the switching on its own?
Alternatively, if you want to avoid dealing with operator type inside the regression function, you can pass a bool that signifies wether you're calculating regression or deriv into there, and have it just return a float, something like linearRegression(isDeriv bool, dps ts.Datapoints, interceptTime time.Time) float64
There was a problem hiding this comment.
I like separate linearRegression since it makes it easy to test. My above comment about having a separate func instead duration should fix this problem.
| func linearRegression(dps ts.Datapoints, interceptTime time.Time) (float64, float64) { | ||
| var ( | ||
| n float64 | ||
| sumX, sumY float64 |
| sumX, sumY float64 | ||
| sumXY, sumX2 float64 | ||
|
|
||
| nonNaNCount int |
| nonNaNCount++ | ||
|
|
||
| // convert to milliseconds | ||
| x := float64(dp.Timestamp.Sub(interceptTime).Nanoseconds()/1000000) / 1e3 |
There was a problem hiding this comment.
I think this converts it to seconds, not millis. Can you just do float64(dp.Timestamp.Sub(interceptTime).Seconds())?
src/query/functions/temporal/base.go
Outdated
| @@ -360,7 +361,7 @@ func (c *baseNode) sweep(processedKeys []bool, maxBlocks int) { | |||
|
|
|||
| // Processor is implemented by the underlying transforms | |||
| type Processor interface { | |||
| Process(values ts.Datapoints) float64 | |||
| Process(values ts.Datapoints, alignedTime time.Time) float64 | |||
There was a problem hiding this comment.
call it evaluationTime ?
| ) | ||
|
|
||
| type linearRegressionProcessor struct { | ||
| duration float64 |
There was a problem hiding this comment.
instead of this, store a function here which takes in a slope and intercept, returns a float. This way you don't have to compare Op in Process.
|
|
||
| var slope, intercept float64 | ||
|
|
||
| if l.op.operatorType == PredictLinearType { |
There was a problem hiding this comment.
I like separate linearRegression since it makes it easy to test. My above comment about having a separate func instead duration should fix this problem.
benraskin92
force-pushed
the
braskin/linear_regression
branch
from
fcdd004 to
57016fb
Compare
| var ( | ||
| n float64 | ||
| sumX, sumY float64 | ||
| sumXY, sumX2 float64 |
| n float64 | ||
| sumX, sumY float64 | ||
| sumXY, sumX2 float64 | ||
|
|
| } | ||
|
|
||
| valueCount++ | ||
|
|
| } | ||
|
|
||
| slope, intercept := linearRegression(dps, evaluationTime, l.isDeriv) | ||
|
|
benraskin92
force-pushed
the
braskin/linear_regression
branch
from
13e976a to
281f999
Compare
No description provided.