-
Notifications
You must be signed in to change notification settings - Fork 75
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
SP: strip unlearned columns: removed #286
Conversation
return number of cols removed/stripped. and print if > 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR makes SP::stripUnlearnedColumns() return the number of columns pruned, which is a useful improvement itself. Not breaking the API (as it was void, so no return usage)
In #281 it's suggested that the method is actually useless, which this PR aims to test, and indeed it never prunes anything.
Options:
- just this improvement with return value (remove the print)
- remove the method (I agree)
Happy saint David's day, @dkeeney @ctrl-z-9000-times @cogmission @david-ragazzi ! 🎉 🎈 🥂 Thanks for your ideas and work!! |
Thank you Breznak for doing this. This is one less thing for users to worry about! |
@@ -57,4 +57,4 @@ are ignored. PR #271 | |||
|
|||
* Removed all matrix libraries. Use the `Connections` class instead. PR #169 | |||
|
|||
* Removed `void SpatialPooler::stripUnlearnedColumns()` as unused and not useful (did not effectively remove any columns). https://github.com/htm-community/nupic.cpp/pull/286 | |||
* Removed `void SpatialPooler::stripUnlearnedColumns()` as unused and not useful (did not effectively remove any columns). PR #286 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ctrl-z-9000-times just a mention, next time I'd prefer the full URL format. To a user reading the document in browser, it both shows as "#286", but the #286
breaks if you copy the code to a repo, or what not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that is the case then you should update all of the cross references in the API_Changelog. I changed it to a short link for consistency with the rest of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update all of the cross references in the API_Changelog
well, I probably don;t care enough to do that now. Was just saying for next time...
removed strip
Fixes: #281