forked from numenta/nupic.core-legacy
-
Notifications
You must be signed in to change notification settings - Fork 79
Cleanup exceptions #742
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
Merged
Merged
Cleanup exceptions #742
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a188e8b
Log: simplify NTA_ASSERT macro
breznak 6605c05
Log: remove NTA_LDEBUG macro, unused
breznak 0137215
Log: deprecate LogItem
breznak ae66bf1
Exception: implement operator<< and remove LoggingException
breznak 1b46714
Log: remove LogItem as unused
breznak c1f6ed6
Network.setLogLevel fix in bindings
breznak 34f9b6f
Network: make setLogLevel static
breznak 4560ce7
TMRegion: remove the hack for NTA_DEBUG
breznak 2149e83
WIP trying to fix linked regions in TMRegion
breznak fac5da7
Merge branch 'master_community' into cleanup_exceptions
breznak 2552213
TMRegion: compute all outputs
breznak 17fdb7c
Made the LogLevel variable thread local
dkeeney File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We talked about always generating all outputs, even if no connections are made. So this new function would not be needed.
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.
I'm having second thoughts about this. I've implemented that in TMRegion and it works fine! (same should then be done for SPRegion).
What we should consider is the tradeoff between ease of use for the user/speed. We're computing outputs that may not be needed (and I'm not sure of it's costly enough to care).
The proble is only with the leaf node (here TM, and not SP) which has no outgoing links. Alternative approach would be a dummy OutputRegion that the user would add after the TM to specify which outputs are linked (=used).
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.
I agree with you. We should not be computing the outputs if they are not used if we can help it.
The problem is when an app gets the input or output buffers directly with region.getOutputData( ) or region.setInputData( ). These could be used on any buffer at any time without the region impl being involved.
Originally, these two functions were not exposed to apps. The apps had to call region.getArrayParameter(name) to access the 'optional' data and in those handlers the region impl had the chance to generate the buffer before returning it.
Perhaps we could add a hook in getOutputData( ) that allowed a region impl to do something before it returned the buffer. That adds yet another complication to the region impl's, but only for those that have optional buffers.
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.
the implementation seems already quite complex to me.
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.
No, the Spec is static and describes what the region will do, not how it is used.
I guess so. We should keep thinking about how we might manage this however.