Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate removal of custom math/*Matrix; #104

Closed
9 of 20 tasks
breznak opened this issue Nov 13, 2018 · 5 comments · Fixed by #149
Closed
9 of 20 tasks

Investigate removal of custom math/*Matrix; #104

breznak opened this issue Nov 13, 2018 · 5 comments · Fixed by #149
Assignees
Labels
code code enhancement, optimization, cleanup..programmer stuff
Milestone

Comments

@breznak
Copy link
Member

breznak commented Nov 13, 2018

There is a huge ammount of code in math/*Matrix that is old, unmaintained and quite untested. Propose removal and replacement :

  • replace with Connections
  • or Eigen Eigen speed #42
  • or keep only the SparseBinaryMatrix

Look at the problem and proposed order of progress:

grep -R 'Matrix' src/nupic/math/ | cut -d: -f1 |sort -u

  • src/nupic/math/Math.hpp
  • src/nupic/math/ArrayAlgo.hpp -- maybe most of its methods will be removed?
  • src/nupic/math/DenseMatrix.hpp -- only one user SDRClassifier, replace with sparse & rm first SDR Classifier without densematrix #170 Sparsematrix removal 2 #169
  • src/nupic/math/NearestNeighbor.hpp -- NN does not have to be in a HTM codebase
  • src/nupic/math/SparseBinaryMatrix.hpp -- heavily used in SP Spatial Pooler: investigate using Connections as backend #93 , Sparsematrix removal 2 #169
  • algorithms/CondProbTable -- rm too? Uses:
    • src/nupic/math/SparseMatrix01.hpp -- not used, rm
  • src/nupic/math/SparseMatrix.hpp -- huge, untested, suprisingly not a base of SparseBinaryM Sparsematrix removal 2 #169
    • used in ArrayAlgo, DenseMatrix, SpatialPooler
  • src/nupic/math/SparseMatrixAlgorithms.hpp -- not used, rm
  • src/nupic/math/SparseMatrixConnections.hpp -- not used, rm
    • src/nupic/math/SegmentMatrixAdapter.hpp -- used only in SMConn
      • py_SegmentSparseMatrix
  • src/nupic/math/SparseRLEMatrix.hpp -- not used, rm
  • src/nupic/math/SparseTensor.hpp -- not sure how needed for Py - implemented (but reverted) here: bfb58df , Sparsematrix removal 2 #169
    • Domain
    • Index
    • PyBindSparseTensor
  • math/Math.hpp:Gaussian_2D -- not used, further cleanup Math.hpp

Related:

Prerequisities:

@breznak breznak added the code code enhancement, optimization, cleanup..programmer stuff label Nov 13, 2018
@breznak breznak added this to the cleanup milestone Nov 13, 2018
@dkeeney
Copy link

dkeeney commented Nov 13, 2018

Hmmm, if that is exposed to Python code it might be used more than you think.

@breznak
Copy link
Member Author

breznak commented Nov 13, 2018

f that is exposed to Python code it might be used more than you think.

Good call, but no. SparseMatrix is used everywhere, Dense only in that one class.
I'd like to:

  • rid off Dense
  • (optionally) replace Sparse with Eigen::SparseMatrix

@dkeeney
Copy link

dkeeney commented Nov 13, 2018

  • rid off Dense

Yes.

  • (optionally) replace Sparse with Eigen::SparseMatrix

Don't know enough about Eigen to tell you if it is a good idea.

@breznak breznak changed the title Investigate removal of math/DenseMatrix Investigate removal of custom math/*Matrix; Dec 1, 2018
@breznak breznak self-assigned this Dec 8, 2018
@breznak
Copy link
Member Author

breznak commented Dec 10, 2018

Reopening to continue further cleanup after #93
Next target: bfb58df

@breznak
Copy link
Member Author

breznak commented Dec 17, 2018

I will try to resume this work and rm further more, given #153 is merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code code enhancement, optimization, cleanup..programmer stuff
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants