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

Fix SDR classifier Region #3669

Merged
merged 6 commits into from
Jun 7, 2017
Merged

Conversation

ywcui1990
Copy link
Contributor

@ywcui1990 ywcui1990 commented Jun 5, 2017

Fixes: #3651
Summary

  • handle multiple categories in sdr classifier
  • call sdr classifier once in sdr classifier region
  • add unit test of sdr classifier class to test multiple category prediction
  • add integration test of sdr classifier region

@scottpurdy This is a fix for the broken SDR classifier. If this looks good to you, I will make the same change in nupic.core

# ensures that recordNum increases monotonically
if len(self._patternNZHistory) > 0:
if recordNum < self._patternNZHistory[-1][0]:
raise ValueError("the record number has to increase monotonically")
Copy link
Member

Choose a reason for hiding this comment

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

Add to the docstring:

:raises: (ValueError) when record number does not increase monotonically.

Copy link
Contributor

@scottpurdy scottpurdy left a comment

Choose a reason for hiding this comment

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

Looks good to me once you address @rhyolight comment

@scottpurdy
Copy link
Contributor

@ywcui1990 - are you ready to merge this? I want to revalidate #3665 once this is merged to make sure the two changes are compatible.

@ywcui1990
Copy link
Contributor Author

@scottpurdy I am ready to merge this. I will push a separate PR in nupic.core. I expect this PR to break regression tests since it affects classification/prediction accuracy. Do you know why the regression tests pass with this PR?

@scottpurdy
Copy link
Contributor

I believe the regression tests only run on a nightly for master, not PRs. @rhyolight - is that right?

@scottpurdy scottpurdy merged commit c42c8bb into numenta:master Jun 7, 2017
@rhyolight
Copy link
Member

@scottpurdy Yes, it's nightly on master only.

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.

SDR Classifier and region don't handle multiple calls correctly
3 participants