Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PushFixed function for static dataset size #10

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

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.

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.


// PushFixed method will keep the size of the Data vector constant.
// Oldest data points will be evicted as points are added.
// WARNING: Mixing Push() and PushFixed() will result in failure!
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