Skip to content

Conversation

@Ropes
Copy link

@Ropes Ropes commented Oct 30, 2015

Instead of Push simply appending data points to ever expanding Go slices; this change
will utilize GoVector's PushFixed function to keep the array size constant as data points
are added by dropping the oldest event. The goal is to keep memory allocations for long
running processes constantly evaluating anomalies consistent.

eg

anomalyzer.Data: [ 1.0, 2.0, 3.0] 
err := anomalyzer.PushFixed(5.0)
anomalyzer.Data: [2.0, 3.0, 5.0]

Mixing Push and FixedPush for a anomalyzer data set will result in failures. The problem stems from the len of an array changing, but the cap not also being updated. There might be a easy fix another change will need to be made to GoVector.

Ropes added 5 commits October 30, 2015 10:31
Instead of push simply appending data points to expanding Go slices; this change
will allow a rolling window dataset to be analyzed for anomalous
signals.
Mixing Push and FixedPush will result in errors, tests prove this
problem(not committed yet). I'll leave that for another day when there's
free time.
@allisonmorgan
Copy link
Contributor

I really like this method and I almost wonder if it would be useful enough for others that an AnomalyzerConf option should be something like Cap. So that if Cap is unspecified, then we just default to the Push() option, otherwise if len(data) > Cap we start to PushFixed() (or whatever doesn't result in failures). @drewlanenga?

@Ropes
Copy link
Author

Ropes commented Oct 30, 2015

I agree, I'd like to have a safe usage of both intermixed, or configurable like you state. However it might just need a re-evaluation of my GoVector code to fix the problem.

Changed the PushFixed array code to not break if the array was expanded
by an external call to the standard Push function. The two functions can
now be called in a mixed fashion, and PushFixed will still attempt to
use as little memory as possible.
@Ropes
Copy link
Author

Ropes commented Oct 30, 2015

@drewlanenga will need to merge this PR into GoVector in order for the tests I added to pass.

@Ropes
Copy link
Author

Ropes commented Nov 6, 2015

Took me a little bit to figure out why it's failing.. It's because my PR on GoVector hasn't been merged yet.

README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

If Push() and PushFixed() no longer conflict, probably should remove this warning and the one below, right before the definition of PushFixed().

@allisonmorgan
Copy link
Contributor

Otherwise 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants