From NuPIC: Can inhibitColumnsGlobal() execute without sort() ? #354 (#365)

[smadasu]

Fixes #354

Here I am using streams. This is not a replacement for nupic python argpartition function. I have measured the performance gain with this change. It gives atleast 4000% performance gain. Please review

[cogmission]

Hi @smadasu !

Sorry for the delayed response, I was away from my computer yesterday eve. I am anxious to see the difference!

The community has a contribution process which I can help you out with here. The 3 "x"'s above have to do with this process; and we must satisfy these measures before we can consider merging your contributions.

1. CL (Contributor's License) - Please fill out the form at this link. This will then be processed by Numenta (@rhyolight might not be able to get to this until Monday, so I appreciate your patience on this due to it being the weekend). But while you're waiting, you can...
2. reference the "issue" your PR solves by editing your description above to include the words

Fixes #354

on a separate line at the top... This will fix the 2nd issue entitled, "Fixes Issue Validator". All PR's in Numenta.org must be associated with an "Issue" for tracking.

"3". We like for all code to be covered by tests or at least have the coverage remain constant or preferably increase. We enforce that with this 3rd requirement which does a scan of the ratio of tested lines of code to the total count of lines using the Coveralls tool. So we need you to add a test to your code in htm.java/src/test/java/org/numenta/nupic/algorithms/SpatialPoolerTest.java.

Cheers @smadasu and thank you for your hard work!

David

[smadasu]

Hi David,
I have already submitted the CL yesterday. May be it will be cleared on Monday. I have updated PR description. As far as the code coverage, I have not added any new code. There was already a test covering the method I changed. Where I have taken out the need for ArrayUtils class, since it is not necessary. Which I think is the reason for code coverage going down.
Thanks
Sreenath

[cogmission]

Hi Sreenath,

Take a look at your comment now? ;-) That is what I meant... (You will see that the line now has a "green check".

Re: [No new Code but Coveralls complains] - Sorry dude, we must make Coveralls Coverage stats always go up, so you will have to submit another test of something that is not already tested. I know its a pain but we all have been through this! Lol!

Try going to the coveralls website by clicking the "Details" link where the line "X coverage/coveralls - Coveralls decreased..." is. Then once there, navigate to the most "uncovered code and add some tests" (That will make it easy to do).

@rhyolight please find this...

Cheers,
David

[smadasu]

Fixes #354
Updated ArrayUtilsTest for code coverage

[cogmission]

Hi@smadasu,

A couple of things while we wait for you CL. Can you make sure you are using spaces instead of tabs, and that the indent is set to 4 spaces instead of 8? Also, the title of this PR should keep the descriptive title, you only have to put "Fixes ###" in your comments not the title.

Once you fix the indentation and make sure you have no tabs, then everything else looks good 👍

[smadasu]

Fixes #354. I have updated the spaces.

[cogmission]

@rhyolight please do not merge this. Just approve the CL. I need to vet the speed increase.

[rhyolight]

CL approved.

[cogmission]

Awesome thanks @rhyolight

[smadasu]

Hi @cogmission,
What is the update on this? ARe you able to test performance?
Thanks
Sreenath

[cogmission]

Hi @smadasu,

I will look at it today. I'm sorry I've been unusually swamped with work while at the same time under the weather due to weird sleep patterns. Usually this stuff has a much faster turn around...

Cheers,
David

[cogmission]

@rhyolight @fergalbyrne @subutai @chetan51

Below you can see the enormous difference in performance. I omit 18 of 20 iterations for brevity's sake.

This was tested using HTM.java's benchmark harness (No NAPI).
Each iteration runs the 1000 inputs through a ScalarEncoder->SpatialPooler.

Before: (SP with Global Inhibition)

# Run progress: 30.00% complete, ETA 00:17:02
# Fork: 10 of 10
# Warmup Iteration   1: SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
8.764 ms/op
# Warmup Iteration  19: 7.826 ms/op
# Warmup Iteration  20: 7.809 ms/op
Iteration  19: 7.760 ms/op
Iteration  20: 7.806 ms/op

With @smadasu 's fix: (SP with Global Inhibition)

# Run progress: 30.00% complete, ETA 00:17:03
# Fork: 10 of 10
# Warmup Iteration   1: SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
5.033 ms/op
# Warmup Iteration  19: 3.795 ms/op
# Warmup Iteration  20: 3.789 ms/op
Iteration  19: 3.835 ms/op
Iteration  20: 3.834 ms/op

///////////////////////////////////

Conclusion:

The Streams API use in inhibitColumnsGlobal() is about a 40 - 50% performance increase!

[cogmission]

Good work @smadasu !

[smadasu]

Fantastic @cogmission !!! I am glad that you could verify the performance boost. Thank you very much for your help. Looking forward to working with htm.java for many more fixes and enhancements. 👍😄

[cogmission]

@smadasu Why the change? I'm just curious if there's something that you've seen needs fixing that would help improve HTM.java? I'm open if you want to talk? :-) (cognitionmission@gmail.com)

[smadasu]

Its just that by using JDK8 streams API, we can boost performance. Just wanted to show case it that's all :)

[cogmission]

@smadasu Can you email me?

[smadasu]

What do you want me to email about?

[cogmission]

@smadasu I was saying that so I could get your email and write you about something...?