SDR Classifier without densematrix

[breznak]

as DenseMatrix is to be removed.
Uses map<UInt, map<UInt, Real>> instead.

• implement WIP parts
• fixes bug in upstream softmax
• measure SDR Classifier performance in Hotgym benchmark -> moved to Real-life benchmark: Hotgym example using C++ algorithms #30

Blocks more cleanup in #169

[breznak]

First draft, looking for comments, esp for Matrix typedef.

• already compiles
• c++ tests fail on SDR Classifier (expected)
  • py tests pass (no testing :( )

[breznak]

I'm looking for comments on implementation of this 2D matrix. It's a mapping from 2d [UInt][UInt] -> Real64;

• used to be DenseMatrix
• now considering:
  • map<int, map<int, double>>
  • Eigen: Matrix<Real64, Dynamic, Dynamic>
  • plain Real64[][] //we sometimes have to grow it
  • vector<vector>

I'm inclining to eigen::Matrix as we need here concept of rows, cols.

[breznak]

for now implemented using native map<int, map<int, double>> map2d;
Eigen would be optional improvement for the future, if needed.

[ctrl-z-9000-times]

I like this matrix type. I prefer the standard library types because they're understood by everyone.

[dkeeney]

I agree std:: classes are always good but I don't have any experience with eigen so I cannot say which is better.

[breznak]

RFC on softmax_()

[breznak]

@htm-community/nupic-cpp I'd like your help reviewing this softmax_ function. I think I found an error in upstream implementation. It is not a big deal (didn't even have a test to spot it, but let's make sure it's right)

• I'm reworking SDRClassifier, and its softmax_()

• wrote test SDRClassifierTest::testSoftmax to compare results (with results obtained from master) -> and we diff!

  • added debug statements
  • backported test and debug to master, you can view it at temporary PR WIP DONT MERGE, for review only: Tmp softmax master #171

• the "original" softmax_ version from master uses:

  • range_exp() from math/ArrayAlgo.hpp
    • Exp() from math/Math.hpp
  • accumulate() - I'm not sure if std::, or something from SparseMatrix::accumulate

Debug results from master:

max=2.018
in=0
in=1
in=1.337
in=2.018
in=1.1
in=0.5
in=0.9
exp=0.132921
exp=0.361317
exp=0.506111
exp=1
exp=3.00417
exp=1.64872
exp=2.4596
sum=9.11284
div=0.0145861
div=0.0396492
div=0.0555382
div=0.109735
div=0.329663
div=0.180923
div=0.269905
[       OK ] SDRClassifierTest.testSoftmax (1 ms)
[----------] 1 test from SDRClassifierTest (1 ms total)

and from this PR:

max=2.018
in=0
exp=0.132921
in=1
exp=0.361317
in=1.337
exp=0.506111
in=2.018
exp=1
in=1.1
exp=0.399317
in=0.5
exp=0.21915
in=0.9
exp=0.326933
sum=2.94575
div=0.045123
div=0.122657
div=0.171811
div=0.339472
div=0.135557
div=0.0743953
div=0.110985
[  FAILED  ] SDRClassifierTest.testSoftmax

 1 FAILED TEST

You can see a strange behavior in the old/master implementation after "2.018 aka max elemen aka where exp=1":
the old function gives exp > 1, while our does return < 1 (as it should). Because we normalized the values to the max element( x[i]-maxVal) , all values should be <= 1.

TL;DR:

• not too critical (no test even before)
• I think ours new is correct, just want to doublecheck

[breznak]

I went ahead with our version

[breznak]

@ctrl-z-9000-times @dkeeney this is now ready, please review when you have time

[dkeeney]

I don't see anything I object to. Again I am not very familiar with these modules so I have to rely on you two to get it right. :)

[dkeeney]

agree, we don't need to be backward compatible with old versions of serialization.

[dkeeney]

I agree std:: classes are always good but I don't have any experience with eigen so I cannot say which is better.

[dkeeney]

Now that I have a fix for Circle CI PR #176 , perhaps we should push this one more time after the fix is in the master so that we can check this PR in debug mode.