Removal of obsolete/non-essential/dead-code from nupic

[david-ragazzi]

Here I list a serie of possible removals for make the nupic repo clearer and easy to follow and maintain as discussed in ML. Some of these removals should also be aplied to nupic.core in case of equivalent code (numenta/nupic.core-legacy#354).

---

nupic/research:

• Dead code (DataGenerator.py, ObjDiff.py, TrainingHooks.py, TrainingHooks.py, anomalyzer.py). These dead codes are used in anywhere (except in their tests/examples). Issue: Remove dead code from nupic/research #1857, Fixed by: Removed dead code from nupic/research #1858
• Obsolete implementations of SP (FDRCSpatial2.py, and fdrutilities.py): Do we really need this old implementation as spatial_pooler is its replacement?. Another point, fdrutilities.py has a lot of dead code. Issue: Remove obsolete implementations of SP #1864, Fixed by: Remove FDRCSpatial2 #1872
• Derived implementations of SP (flat_spatial_pooler.py). Issue: Remove flat spatial poolers #627, Fixed by: Remove FlatSpatialPooler #2056
• Derived implementations of TP (TPTrivial.py and TrivialPredictor.py): I've checked the use of these classes in code, and I found that are little used. Issue: Remove derived implementations of TP #1865, Fixed by: Remove derivated implementations of TP #1873
• Vision code. Issue: Move vision code to appropriate repo nupic.vision #1863

It seems that TP is the big problem to a clean repository as it has several derived classes and hacks to keep consistency between them. As temporal_memory gets mature, we could remove TP and its inheritance and so the repository finally would get nice and clean. By the way, while TP is not fully removed, some derived classes could be removed right now (TPTrivial, fast_temporal_memory, etc).

[breznak]

just to clarify, is the goal of TM to do temporal predictions? (what TP does), or is it "just" a sequence memory (justifying functionality for both TP/TM) ?

[cogmission]

The current TM does predictions for one step. The Old TP does predictions
for more than one step (and in this way had/has rudimentary Temporal
Pooling - which has advanced in its definition to include stabilization,
therefore pooling as a concept was pulled out of the TP).

I may be wrong - Chetan?

David

On Mon, Feb 23, 2015 at 8:32 PM, breznak notifications@github.com wrote:

just to clarify, is the goal of TM to do temporal predictions? (what TP
does), or is it "just" a sequence memory (justifying functionality for both
TP/TM) ?

—
Reply to this email directly or view it on GitHub
#1862 (comment).

We find it hard to hear what another is saying because of how loudly "who
one is", speaks...

[subutai]

👍 for removing FDRCSpatial2, TPTrivial, TrivialPredictor.py and the unused code in fdrutilities.py.

TP and TP10X are important right now - they are the main ones used in production and by the OPF. TP10X is an optimized version of TP. There are tests to make sure they are identical in functionality. They are somewhat redundant with temporal_memory.py but until we have a fast C++ version of that we can't switch over.

@chetan51 can probably comment on the others.

[david-ragazzi]

👍 for removing FDRCSpatial2, TPTrivial, TrivialPredictor.py and the unused code in fdrutilities.py.

Thanks!

I created issues for them. I also re-organized the checklist to reduce the number of issues and future PRs.

Furthermore, I created a mirror super issue in nupic.core: numenta/nupic.core-legacy#354

[chetan51]

@cogmission The old temporal memory implementation (TP.py) also only made one prediction (of the next time step). The temporal pooling functionality in it (making predictions of multiple time steps in advance) is deprecated.

[subutai]

Someone should keep track of how many lines and files NuPIC currently has, and then check again when we do the cleanup release. It will be cool to see how much we actually remove!

[cogmission]

👍 On before/after tracking :)

On Tue, Feb 24, 2015 at 4:17 PM, Subutai Ahmad notifications@github.com
wrote:

Someone should keep track of how many lines and files NuPIC currently has,
and then check again when we do the cleanup release. It will be cool to see
how much we actually remove!

—
Reply to this email directly or view it on GitHub
#1862 (comment).

We find it hard to hear what another is saying because of how loudly "who
one is", speaks...

[breznak]

https://github.com/numenta/nupic/graphs/contributors?from=2015-01-01&to=2015-02-25&type=d

[david-ragazzi]

I think this cleanup release will be cleaner in sense of understand the CLA algorithms, not in size (although some decreasing of lines). The files that are being removed are not too big, but their presence confuse a lot of users (even expert guys).. I believe this is the big deal..

[david-ragazzi]

@chetan51 Could you say us whether fast_temporal_memory.py is critical and should be kept?

[chetan51]

Could you say us whether fast_temporal_memory.py is critical and should be kept?

It is critical, because it's the version of temporal memory that one would use in a production setting for performance reasons.

[david-ragazzi]

It is critical, because it's the version of temporal memory that one would use in a production setting for performance reasons.

I got it. Then once @rcrowder pull the c++ version of TemporalMemory to nupic.core we could remove it, right?

[chetan51]

Currently, the approach I have in mind is:

temporal_memory.py is a readable implementation that is easy to experiment with.
fast_temporal_memory.py is a fast python implementation.
TemporalMemory.cpp (to be created) is a pure C++ implementation to make nupic.core standalone.

After creating TemporalMemory.cpp, if we create a python version that interfaces with TemporalMemory.cpp (analogous to TP10X2.py), then we might be able to remove fast_temporal_memory.py.

[david-ragazzi]

After creating TemporalMemory.cpp, if we create a python version that interfaces with TemporalMemory.cpp (analogous to TP10X2.py), then we might be able to remove fast_temporal_memory.py.

Yes, I agree. I forgot mentionate that TemporalMemory.cpp should be available in nupic.bindings.algorithms.

[scottpurdy]

Obsolete implementations of TP (TP.py, TP10X2.py, and their crutchs: TP_shim.py and TemporalMemoryShim.py): When temporal_memory gets more mature, could TP.py and TP10X2.py be removed? If so, then TP_shim.py and TemporalMemoryShim.py will not make more sense.
Derived implementations of TM (fast_temporal_memory.py): Is this derived class very important?

Everything mentioned in there needs to be kept. Can you remove those two bullets from the description?

[david-ragazzi]

Everything mentioned in there needs to be kept. Can you remove those two bullets from the description?

I was waiting a @rcrowder feedback about C++ TM port. Depending his progress we could remove TP and its childreen in the "cleanup" milestone which this super issue aims. But it seems that this job requires a lot of work and an milestone only for it. What do you guys think we have a milestone only for "Removal of old TPs and fast_temporal_memory"?

[scottpurdy]

@david-ragazzi - that is definitely outside the scope of a clean up PR. It is possible we would go through a bunch of effort to validate the new TM implementation and find out we still need to keep the TP/TP10X2 implementations because they are better for certain problems.

If you really want to work on that then perhaps @subutai can outline the tasks we would go through to see if the new TM implementation can completely replace TP/TP10X2. This isn't a priority for Numenta folks so someone from the community would have to do it. And like I said, the outcome might be validation that TP/TP10X2 are better for some important applications as is our current belief.

A couple notes on the other items:

• The shims are to make it easy to run different implementations side-by-side, as would be necessary for determining if we can delete the TP/TP10X2 implementations.
• The "fast_temporal_memory" is functionally the same as "temporal_memory.py" but it uses a C++ data structure to speed things up a lot. We will keep both options.

[david-ragazzi]

If you really want to work on that then perhaps @subutai can outline the tasks we would go through to see if the new TM implementation can completely replace TP/TP10X2. This isn't a priority for Numenta folks so someone from the community would have to do it. And like I said, the outcome might be validation that TP/TP10X2 are better for some important applications as is our current belief.

@scottpurdy We would have to try anyway.. Even so some renaming is necessary and perhaps TP and TP10X2 could become a single class (well, just speculations)..

About this:

The "fast_temporal_memory" is functionally the same as "temporal_memory.py" but it uses a C++ data structure to speed things up a lot. We will keep both options.

@chetan51 said:

After creating TemporalMemory.cpp, if we create a python version that interfaces with TemporalMemory.cpp (analogous to TP10X2.py), then we might be able to remove fast_temporal_memory.py.

By the way, as I said we have to try.. No way reject improvements if we not try it at least. Do you agree?

[scottpurdy]

David, you need to understand what the different components do before you can make a judgement to remove them or not.

TP and TP10X2 cannot be combined. The former is a mostly-Python implementation and the latter is an entirely C++ implementation. They are functionally identical (we have explicit tests to enforce this) but the mostly-Python version is easier to experiment with while the C++ one is faster / better for production.

And similarly, fast_temporal_memory contains a subclass of the new temporal memory that uses a fast C++ data structure. So if we had a completely-C++ implementation of the temporal memory then yes, we could swap from the hybrid Python class to the bindings for the complete C++ implementation.

The general philosophy is that we want a full C++ implementation and a full Python implementation with tests to ensure they are functionally identical. This allows for easy experimentation in Python with fast C++ implementations for production use. But sometimes we deviate from this when we can move small pieces of the Python to C++ for a significant speed up while still having most of the code in Python for easy experimentation. It seems like the different implementations and strategies make you uncomfortable but we aren't going to artificially restrict ourselves to a single implementation or to only pure Python or pure C++ implementations.

By the way, as I said we have to try.. No way reject improvements if we not try it at least. Do you agree?

I'm not sure what you are referring to.

[david-ragazzi]

Thanks, @scottpurdy. I think I was a little hasty about it. Now I have better idea of what you mean and why you guys want preserve them.

In other words, we have TP (i.e. temporal_pooler.py) which is similar to temporal_memory.py (pure python impl) in same way that TP10x2 (AKA fast_temporal_pooler.py??) is similar to fast_temporal_memory.py (optimized python impl with C++).

[scottpurdy]

Yup!

[david-ragazzi]

Well, I created an issue which suggest we uniformize these class names. Hopefully once their serialization is done, we can rename them accordingly:

#1898

[rhyolight]

Subtasks are not complete, even though they were checked int he description. I've unchecked the incomplete subtasks and reopened this ticket.

[david-ragazzi]

Vision code. Issue: #1863

@breznak Can I leave this issue as independent issue in order to I close this super issue (which is quite related to CLA algorightms)?

[rhyolight]

All subtasks complete.