nupic-legacy icon indicating copy to clipboard operation
nupic-legacy copied to clipboard

Incorrect terminology in spatial_pooler.py

Open subutai opened this issue 8 years ago • 11 comments

The file spatial_pooler.py contains two helper classes, CorticalColumns and BinaryCorticalColumns. These names are incorrect as these should refer to mini columns. There are also numerous references to cortical columns in the comments.

The term "cortical column" means something completely different in neuroscience. This issue is particularly relevant since in the new theory work, we rely on the correct meaning of "cortical column".

This should be an easy change for someone to implement. Other than this file and some tests, nothing else in NuPIC uses these helper functions.

subutai avatar Aug 11 '16 20:08 subutai

@subutai @rhyolight

I made changes and referenced this issue. I want to know If I am on the right path.

arhik avatar Aug 14 '16 10:08 arhik

@arhik Thanks for your work, but I don't think we need to change all "column" references into "miniColumn" references. For example, I don't think this method signature needs to change:

def update(self, columnIndex, vector):

In this case, columnIndex is generic and works.

I think Subutai wanted to make sure that all references to "cortical column" were changed to "mini column", and that's about all. No need to prepend mini in from of any column reference.

Also, can you turn your changes in to a pull request? Here some help: https://github.com/numenta/nupic/wiki/Developer-workflow

rhyolight avatar Aug 15 '16 17:08 rhyolight

Thank you. Rereading Subutai comments with your feedback helped a lot. Definitely I was over thinking all this time. I was dealing with questions like what's mini column ( I was aware of columns); will you be eventually working on hyper column ( hyper column popped when I googled for mini column), and subutai's statement like 'we rely on correct implementation of cortical columns' confused me even more.

I tried not to bug you with many questions on weekend and started working on it by implementing various possibilities in different commits. I referenced commits incrementally and separately. I only changed every 'column' to 'minicolumn' in the last commit because at one point even that sounded right. I hope you understand. I should have mentioned that ahead of referencing commits.

Based on your feedback .. my first commit itself is wrong. class names should be MiniColumns, BinaryMiniColumns. I will rework on this soon.

arhik avatar Aug 15 '16 21:08 arhik

@arhik I think your confusion is exactly the point of this issue :-)

At the end this should be a relatively simple PR. It is only to rename the classes CorticalColumns and BinaryCorticalColumns, and change text from "cortical column" to "mini column" in the comments. The phrase "cortical column" is incorrectly used in this file.

The word "column" by itself is ok for now. So in the main spatial pooler class, if it just says "column" (not cortical column) then just leave it as-is.

And yes, class names should be like SparseMatrixMiniColumnAdapter and not like SparseMatrixCorticalMiniColumnAdapter

Hope this helps.

subutai avatar Aug 15 '16 21:08 subutai

@subutai @rhyolight Got it and working on it. :-)

arhik avatar Aug 15 '16 23:08 arhik

Now newbie tag is making sense. I back off from the commit which will fix setPotential bug in spatial pooler. I didn't notice that it was referenced by rhyolight already. Just mentioning it because PR referencing it is still open making me think you are waiting for my PR which will do the same.

arhik avatar Aug 16 '16 18:08 arhik

@subutai @arhik See my comments on the PR. Sorry guys, we can't work on this until we've switched to the new serialization technique, which is impending work.

rhyolight avatar Aug 16 '16 21:08 rhyolight

That's fine. It was just editing the doc. I learned something atleast. @rhyolight alone could have done it in five or 10 minutes possibly. I like the idea of tagging the tasks as newbie; priority:# etc... , also the Idea of open research and your attempts to involve more people into this. :+1: for open research, I will all universities does this.

So there are portions of code which needs to be ported to or implemented using capnproto instead of pickle.

Off topic request: Ignore me if my request is highly ambitious, but I cant stop myself to ask. Is there anyway I can work closely with someone on a particular topic instead of picking some random topic/issue and fix it. I am not complete stranger to nupic. nupic is like an obsession to me since last year. My last contribution was 'hello_tm.py' and lost track of nupic since then because of my own coursework. Though hello_tm.py was pretty small contribution I had good knowledge of nupic and nupic.research code sections then. I want to be back on track and also I might do PhD in computational neuroscience, so any exposure to research would help me. You can just handover jobs as if i am numenta employee and i will work as if i will be fired if job not done. I wont expect anything in return.

When i say someone yuwei is in my head. But it could be anyone. I am tracking his reseach now, thats why. I could help speed up the research/ if I am not good at it nothing would change. I wont slow you down for sure.

arhik avatar Aug 17 '16 14:08 arhik

@arhik Thanks for your enthusiasm! This is you on HTM Forum, right? I'll start a thread in NuPIC Developers about newbie tasks and we can chat there.

rhyolight avatar Aug 17 '16 14:08 rhyolight

I removed the newbie and P3 labels for now, since we cannot work on this yet.

rhyolight avatar Aug 17 '16 14:08 rhyolight

@rhyolight Yes thats me and thank you.

arhik avatar Aug 17 '16 15:08 arhik