Skip to content

Added fix for potential radius#484

Merged
rhyolight merged 2 commits into
numenta:masterfrom
cogmission:potential_radius_fix
Sep 28, 2016
Merged

Added fix for potential radius#484
rhyolight merged 2 commits into
numenta:masterfrom
cogmission:potential_radius_fix

Conversation

@cogmission
Copy link
Copy Markdown
Collaborator

Fixes #483

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.008%) to 82.452% when pulling f23d139 on cogmission:potential_radius_fix into 820eb63 on numenta:master.

@cogmission
Copy link
Copy Markdown
Collaborator Author

@rhyolight Please accept. This is the potentialRadius fix for HTM.Java

Copy link
Copy Markdown
Member

@rhyolight rhyolight left a comment

Choose a reason for hiding this comment

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

Only comment is to remove all but two warnings. They should only exist where the value could be set.

*
* <b>WARNING:</b> potentialRadius **must** be set to
* the inputWidth if using "globalInhibition" and if not
* using the Network API (which sets this automatically)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for this warning comment on a getter.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok. I removed this one on the "getter"

* the inputWidth if using "globalInhibition" and if not
* using the Network API (which sets this automatically)
*/
defaultSpatialParams.put(KEY.POTENTIAL_RADIUS, -1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this warning is in too many places. It makes sense to put it in the setter or constructor (where the value can be set), but not everywhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I removed this one, but I added it (where it should have been), on the key itself (where it will be looked up).

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.008%) to 82.452% when pulling 831607c on cogmission:potential_radius_fix into 820eb63 on numenta:master.

@rhyolight rhyolight merged commit b99fd45 into numenta:master Sep 28, 2016
@cogmission cogmission deleted the potential_radius_fix branch March 22, 2018 13:46
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.

3 participants