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

Changed distribution keyword, casted some attributes to float, remove… #3784

Merged
merged 1 commit into from Jan 3, 2018
Merged

Changed distribution keyword, casted some attributes to float, remove… #3784

merged 1 commit into from Jan 3, 2018

Conversation

ghost
Copy link

@ghost ghost commented Dec 28, 2017

This fixes #3783, but there were other issues discovered underneath these fixes.

  1. The incoming object to be serialized has the structure self._distribution['distribution']['mean'], self._distribution['distribution']['stdev'] etc, but the write method expects self._distribution['distributionParams']['mean'] (anomaly_likelihood.py line 318-321)

  2. The read method did not expect distribution or distributionParams (anomaly_likelihood.py line 274-279) thus when the serialized object was read back into memory it did not have a matching structure.

  3. The incoming values cannot be serialized by capnproto because they have the type numpy.float64, so we cast them to float (which is a double) (anomaly_likelihood.py line 318-321)

  4. The AnomalyLikelihoodRegion.capnp file uses Float32 (not a double) instead of Float64.

@rhyolight I'm one of the developers working on Grok, we need to use these serialization methods, if you want some time to talk on the phone about these changes I would be happy to do that. There is some confusion here as to how you guys are serializing datetimes, looks like you just save an integer in place of it to preserve order, (why not serialize a timestamp and recreate the datetime object?)
Please do look over this code though, because it throws a fatal KeyError as is. Greatly appreciate all the help given on the forums!

…d setting list

Changed capnproto schema, added distribution key on read method
@rhyolight
Copy link
Member

The CircleCI build hung, so I restarted it. We received @kyle-sorensen's contributor license and we're processing it. I saw it. This is good to merge.

@rhyolight
Copy link
Member

I'm trying to fix the CircleCI build here.

@rhyolight
Copy link
Member

Ignore these two validation failures:

  • Contributor Validator because I've confirmed he has signed, it just is not processed yet
  • continuous-integration/bamboo because we know it is not running on PR, which is ok as long as the other two CI builds pass.

@rhyolight rhyolight merged commit 5ecae91 into numenta:master Jan 3, 2018
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.

Anomalylikelihood write method throws KeyError distributionParams vs distribution
1 participant