New metrics #3
New metrics #3
Conversation
| return nil | ||
| } | ||
|
|
||
| numbers := []float64{} |
There was a problem hiding this comment.
So a couple of questions about the mode.
- Do we need to do two loops? Looks like you could cover this all in a single loop.
- Why do we return an empty loop if the modes are all of the numbers / one? I haven't dealt with modes too much but wouldn't it still be relevant to have the only possible value returned regardless?
If you could add some comments re: mode that'd be good too - in fact we should quickly explain what each of the averages are - I always get them mixed up, so it'd be nice not to have to think too hard.
| @@ -91,3 +183,76 @@ func (a *max) Result() interface{} { | |||
| result, _ := a.amount.Float64() | |||
| return result | |||
| } | |||
|
|
|||
| // Cadinality | |||
| } | ||
|
|
||
| func (a *cardinality) AddDatum(datum interface{}) { | ||
| a.size++ |
There was a problem hiding this comment.
Cardinality should be the total unique elements added, so this should really be a map[interface{}]bool where the values are comparable, and the Result should call len(that_map).
There was a problem hiding this comment.
If you can't just use interface{} because it doesn't consider a == b, then perhaps make an actual interface
There was a problem hiding this comment.
Or you could just do a known type switch, i.e
switch t := datum.(type) {
case string:
a.values[t]++
…
}
| // Even slice? Get the mean of the middle values. | ||
| if len(a.list)%2 == 0 { | ||
| prev, _ := a.list[middle-1].Float64() | ||
| median = (median + prev) / 2 |
There was a problem hiding this comment.
Might be worth doing decimal math here.
| sort.Sort(decimalSortNumerical(a.list)) | ||
|
|
||
| // Determine median value. | ||
| middle := len(a.list) / 2 |
There was a problem hiding this comment.
You could probably do this in an else below, rather than double working.
| @@ -245,8 +246,22 @@ func (p *queryProcessor) measure() { | |||
| // Now add all of the data to the measurer. | |||
| for j := range bucket.sourceRows { | |||
| row := bucket.sourceRows[j] | |||
|
|
|||
| // If measurer is cardinality, we can +1 not worrying about field value. | |||
| if (reflect.TypeOf(m) == reflect.TypeOf(&cardinality{})) { | |||
There was a problem hiding this comment.
You should do this for a type check instead:if _, ok := m.(*cardinality); ok {
But.... this doesn't really apply, since cardinality should be unique, I'm not sure this shortcut works any more.
…assing measuer{} to IsMetricable to determine if metric can run
|
Thanks @snikch - Really appreciate it.
Questions on
I was always under the impression mode was the value(s) that occur/repeat most often within a set. If all values have the same number of occurrences, then they aren't technically a mode? After a quick google, some say they should be considered mode, others say they shouldn't. Now I'm not so sure. hahaha. Thoughts? I'll ask Lance if he has a preference for either option. Examples: If the dataset is
Added comments to each metric. Metricable string values With To resolve, I'm passing the Previously, the type of cell determined if the field was metricable. Passing the measurer, allows us to run certain metrics against certain cells. As it stands, only needed for Do you think this is an alright solution? Maybe I should define what measurers can be used on what cells outside of each XCell implementation? |
|
Going to merge this after our discussion/look over last night. Hope your Xmas party is in full swing!
|
This change removes
avgas a metric and splits it out into three new metrics (mean,medianandmode). Also implemented issum,cardinalityandstdev