Find unused methods, bitrot

[breznak]

By using IDE, or some tool - https://github.com/caolanm/callcatcher
..and remove them

[ctrl-z-9000-times]

We should remove the files:

• src/nupic/os/Regex.cpp
• src/nupic/os/Regex.hpp
• src/test/unit/os/RegexTest.cpp

It contains a single function "match".
Its not used:

$ grep -inrE "include.*regex" ./src/
src/test/unit/os/RegexTest.cpp:28:#include <nupic/os/Regex.hpp>
src/nupic/os/Regex.cpp:26:#include <nupic/os/Regex.hpp>
src/nupic/os/Regex.cpp:30:#include <regex>
src/nupic/os/Regex.cpp:33:#include <regex.h>
src/nupic/ntypes/Sdr.hpp:35:#include <regex>
    ^^^ This is the real regex, from c++11 stdlib  ^^^
src/nupic/engine/Region.cpp:32:#include <regex>

$ grep "match\|regex" src/nupic/engine/Region.cpp
#include <regex>
End of grep output, Region.cpp includes but does not use regex ...

[ctrl-z-9000-times]

I think we should remove the method:

Synapse Connections::minPermanenceSynapse_(Segment segment) const

It's private by the underscore naming convention although technically protected.
It's unused and has no unit tests:

grep -rn minPermanenceSynapse_ ../../src/
../../src/nupic/algorithms/Connections.cpp:357:Synapse Connections::minPermanenceSynapse_(Segment segment) const {
../../src/nupic/algorithms/Connections.hpp:539:  Synapse minPermanenceSynapse_(Segment segment) const;

[ctrl-z-9000-times]

I've noticed several methods by the name printState.

• They appear to be unused
• They appear to identical
• They have the comment "Debugging helper"
• They do not interact with the classes which they're part of.

printState is a function which pretty prints a vector. Maybe this function can be moved to the file "/nupic/utils/vector_helpers"?

[dkeeney]

I assume you are looking at BacktrackingTM?
Those were in the python code that I ported. I think they should be kept because they are useful in debugging any problems with BacktrackingTM. If you want do that function using something in vector_helpers that would be fine too.

[ctrl-z-9000-times]

I assume you are looking at BacktrackingTM?

Oops I didn't look there...

printState is in:

• SpatialPooler
• TemporalMemory
• BacktrackingTM
• Cells4

Of these 4 instances, SpatialPooler & TemporalMemory have the same very simple code, which I suggest moving to "utils/vector_helpers.hpp"

BacktrackingTM and Cells4 have actually useful implementations which are specific to their class. I agree that we should keep these as they are.

[dkeeney]

After PR #196 is merged there will not be any need for types/Types.h as a separate file. I would like to merge Types.h into Types.hpp. Types.h is still included in a few places but they are all C++ code and can use Types.hpp instead.

[ctrl-z-9000-times]

We should delete file: src/nupic/algorithms/Scanning.hpp
Contains single unused function computeAlpha
With the comment: // Performs the time-intensive steps of ScanControlNode.getAlpha
The word ScanControlNode does not appear in this repo aside from this comment.
The copyright is from 2013.

[ctrl-z-9000-times]

src/nupic/algorithms/BitHistory.hpp and
src/nupic/algorithms/BitHistory.cpp
Appear to be unused.

[ctrl-z-9000-times]

src/nupic/os/DynamicLibrary.hpp and
src/nupic/os/DynamicLibrary.cpp
Appear to be unused.

[ctrl-z-9000-times]

bindings/py/cpp_src/bindings/module.cpp
Appears to be unused.

[dkeeney]

@ctrl-z-9000-times The files DynamicLibrary.hpp&.cpp were utilities to allow runtime loading of shared libraries but as of this point this capability is not used. So, yes we can delete it.

The bindings/module.cpp seemed to be an attempt to combine the three binding shared libraries but the .py code is expecting three libraries so we don't need this.

Something else that can now be removed is NodeSet.hpp. It defines a structure declared in Region and RegionImpl but it is never used. We don't have nodes any more.

The Region class is defined in Region.hpp and implemented in Region.cpp, Regionio.cpp, and RegionParameters.cpp. At one time there was a lot of code so that is why there are three .cpp files but now that a lot of code has been removed these three could be combined.

[ctrl-z-9000-times]

We can replace the Timer class with calls to the clock() from <time.h>

clock_t start_time = clock();
elapsed_time = (double)(clock() - start_time) / CLOCKS_PER_SEC;

The clock() function returns the amount of CPU time that the calling process has consumed, which is probably what we want for measuring speed performance. Currently the timer measure the real system time.

We really don't need a whole class to make two standard library calls. But since we have a Timer class: we also have documentation and unit-tests for it. We have unit-tests for a standard library function ...

The method float Timer::getSpeed() can be moved to nupic/utils as a single function.

[dkeeney]

I agree with all of that.

[ctrl-z-9000-times]

So I encountered a potential issue with removing Timer: it's used to profile the code. I'm going to go ahead and remove the profiling code where possible.

The engine measures the run time for each region. This is part of the public API, methods:

void Region::enableProfiling()
void Region::disableProfiling()
void Region::resetProfiling()
const Timer &Region::getComputeTimer() const
const Timer &Region::getExecuteTimer() const

I'd like to remove these. If that's unacceptable then I'd like to at least change the Region::get*Time() methods to return the elapsed seconds instead of a Timer object.

[dkeeney]

I think lapped CPU time is a better way to do profiling.
I have no problem changing this because it does not directly affect the API. At the same time we don't want to loose tools that help us do a better job.

[ctrl-z-9000-times]

tools that help us do a better job.

There are some very good profiling programs which can do a better job at this than humans can, by automatically instrumenting the code instead of us manually instrumenting the code. In specific valgrind works well.

[ctrl-z-9000-times]

I think we can remove the file "bindings/py/cpp_src/bindings/math/Matrix.hpp"

• It is not included anywhere, CMake doesn't include it, python doesn't see it
• It appears incomplete, has throw std::runtime_error("Not implemented");
• All of the other matrix code was removed
• Contains single class Numpy_Matrix. Pybind11 can access the numpy libraries from C++ so this should never be needed.

[ctrl-z-9000-times]

I think we can also remove the file "bindings/py/cpp_src/bindings/algorithms/py_Algorithms.cpp" which contains two functions: getSegmentActivityLevel & isSegmentActive:

• No documentation for either function.
• No tests for either function.
• Appear to do the same thing as the Connections class
• Require the use of a data structure which bundles two indexes & the synaptic permanence?

[breznak]

I think we can remove the file "bindings/py/cpp_src/bindings/math/Matrix.hpp"

good spot. This should definitely be removed, as is obsoleted.

contains two functions: getSegmentActivityLevel & isSegmentActive:

sounds reasonable to me, as we prefer Connections to do the job.

[ctrl-z-9000-times]

Consider removing file src/nupic/utils/stlio - Only included in SP/TM tests (so we could live w/o it)

[ctrl-z-9000-times]

Crossref: SDR replaces VectorHelpers #320
Vector helpers has already been trimmed down to a single function, which the SDR class has an equivalent of.

[ctrl-z-9000-times]

We should remove namespace testing and namespace htm_ext (python bindings). These namespaces are not needed since that code can never be included into another program, because it does not have header files.