Does anyone have opinions on this? Would this extraction remove duplicate code between SP and TM?
My personal “knee-jerk” response is to only pull it out if:
- You feel the boosting or inhibition implementation may change between versions of the SpatialPooler.
- You want to make it easier to experiment with different boosting and/or inhibition algorithms.
- You foresee these “functions” becoming less essential or less used in the future.
- There are opportunities for parallelization of their functions.
- You want to serialize and restore different versions to mix with the state of a given SpatialPooler or experiment with asserting a certain state for research purposes.
- You decide to to pull all state out of the algorithms (which should be done because it increases flexibility and because the algorithms are processes which are distinct from “state”), and so separating the boosting and inhibition can be done to further the modularization.
Ok that’s just off the top of my head…
Cheers,
David
Hello @committer s? @breznak has a proposal, we should at least give it an up or down vote. What is this, the US Congress? 
Please state your opinion on the matter. If no opinions emerge, I’m going to approve the issue for work. If you give a 
please provide a reason.
1 Like
Conceptually, I’m on board.
I can see two ambiguous parts here.
First, someone needs to come up with the integration points. The SpatialPooler will make calls into a Boosting or Inhibition object… when and where? For boosting, the object will probably need to do bookkeeping, and probably needs to know about timesteps. Maybe it just takes in overlaps and boosts them in-place? Part of this proposal is to add extensibility, allowing different boosting or inhibition algorithms to be inserted. Can we come up with an interface to these objects that works for all potential algorithms?
Second, how does this work with pointers and ownership? In C++, using polymorphism means using pointers or references. You can’t pass a Derived into a function expecting a Base – you have to use Derived* and Base*, or Derived& and Base&. The C++ SpatialPooler will hold a reference or pointer to a Boosting and Inhibition object. Who instantiates the object? Who deletes it? Maybe we should use smart pointers? The SpatialPooler could hold a smart pointer to the object, and a setBoostingAlgorithm method could take in a smart pointer? And the SWIG bindings for the setBoostingAlgorithm method could also save it to self.boostingAlgorithm so that the Python garbage collector doesn’t destroy it? (I’m interested in this problem – it would be useful to know how to build pluggable algorithms into the SP, TM, etc.)
I don’t think we’ll be ready to approve this until we have answers to these questions. I don’t know yet if this task is possible.
1 Like
I believe specifically that the column/cell matrix should be pulled out of the algorithms and exist on its own and be a member of the Connections class. The algorithms should “act” on the columns/cells and they should definitely not be split between the SP and TM (i.e. with the SP owning column info, and the TM owning Cell info - this is what you’re experiencing difficulties making things discrete). That’s one part…
Eventually, we could have more than two algorithms acting on this data structure (the columns/cell matrix or whatever type container) and it will begin to get really hairy.
This was proposed over a year ago and we voted it down, so we should not consider this option.
That wouldn’t solve this problem. If the SpatialPooler held exactly one piece of state – a BoostingAlgorithm instance – we’d still have this problem.
Either way, it’s a moot point. Let’s not get into this discussion here please.
A post was split to a new topic: Unifying the columns/cells into a single entity
I agree with @mrcslws and will defer to him on whether there’s a clean way to do this.
There are three ways to make this stuff extensible:
- Support dependency injection for specific logic like boosting. This is nice for well defined sections that may commonly be changed but requires that all versions of the logic have the same interface.
- Make the implementation modular (but not pluggable) so that the logic can be reused in new implementations. This gives more control but requires a little more work to make initial changes.
- Similar to 2 but instead of creating a new class that selectively reuses logic, subclass the original and override the functions that you want to change.
2 and 3 can be used in different cases, depending on the needs, without rearchitecting the implementation to support injecting different logic for some pieces.
This is a good point, and one can implement boosting variations by subclassing the SP. Sure having an independent Boosting class would make experimentation of boosting mechanisms easier, but I’m hesitant to do any refactoring, especially without seeing a proposal or implementation of an SP with a different boosting flavor.
I don’t understand the purpose of this request. IMO it will make the implementations more complex, not simpler. It is not clear to me why we need a standalone class hierarchy and multiple implementations of boosting for a feature we usually turn off. It’s like spending a bunch of dry cleaning money on that old pile of shirts you never wear.
As far as inhibition goes, there is no commonality between the across-column inhibition in the spatial pooler and the within-column inhibition of temporal memory. Again, I don’t see how pulling inhibition into its own class hierarchy will simplify things.
We have a high hurdle for making algorithm changes within NuPIC, but we encourage experimentation outside of NuPIC. An algorithm case should be made for this change outside of NuPIC, possibly in one of the community repos, before we make the code more complex. Someone could easily copy the spatial pooler into another repo and prove why we need multiple boosting algorithms to reside simultaneously in the repo, how this leads to better spatial pooling, and how it makes the code simpler.
2 Likes
Born out of the need to try some experimental changes to boosting and inhibition, I believe recent changes to the pure-python TemporalMemory implementation help with extensibility there. For example, https://github.com/numenta/nupic/commit/d35024d5701a942f7671f716a80d033c5e668212 makes it easier to subclass TemporalMemory, and https://github.com/numenta/nupic/pull/3334 makes it possible to use custom implementations of Connections in TemporalMemory subclasses.