numenta / nupic.core-legacy

Implementation of core NuPIC algorithms in C++ (under construction)
http://numenta.org
GNU Affero General Public License v3.0
272 stars 276 forks source link

SparseMatrixConnections::createSegments() crashes #1444

Closed Thanh-Binh closed 5 years ago

Thanh-Binh commented 5 years ago

Hi all, I am using SparseMatrixConnections for createSegment() and after that for growSynapsesToSample() like

void test()
{
   std::vector<UInt32> newSegments(newSegmentCells.size(), 0);        connections->createSegments(newSegmentCells.begin(), newSegmentCells.end(), newSegments.begin());
connections->growSynapsesToSample(&(newSegments[0]), &(newSegments[0])+newSegments.size(), &(anchorInput[0]), &(anchorInput[0])+anchorInput.size(), numNewSynapses, initialPermanence, random);
return;
}

It crashes when the size of new segments > 1 because the pointer of newSegments is increased in the function createSegments(), so that by finishing test(), the program tries to destruct the vector and get an error like: free(): invalid pointer.

Do you have any idea for solving this problem? Thanks

  template <typename InputIterator, typename OutputIterator>
  void createSegments(InputIterator cells_begin, InputIterator cells_end,
                      OutputIterator segments_begin) {
    assert_valid_cell_range_(cells_begin, cells_end, "createSegments");

    InputIterator cell = cells_begin;
    OutputIterator out = segments_begin;

    const size_type reclaimCount = std::min(
        destroyedSegments_.size(), (size_t)std::distance(cell, cells_end));
    if (reclaimCount > 0) {
      for (auto segment = destroyedSegments_.end() - reclaimCount;
           segment != destroyedSegments_.end(); ++cell, ++out, ++segment) {
        segmentsForCell_[*cell].push_back(*segment);
        cellForSegment_[*segment] = *cell;
        *out = *segment;
      }

      destroyedSegments_.resize(destroyedSegments_.size() - reclaimCount);
    }

    const size_type newCount = std::distance(cell, cells_end);
    if (newCount > 0) {
      const size_type firstNewRow = matrix.nRows();
      matrix.resize(matrix.nRows() + newCount, matrix.nCols());
      cellForSegment_.reserve(cellForSegment_.size() + newCount);

      for (size_type segment = firstNewRow; cell != cells_end;
           ++cell, ++out, ++segment) {
        segmentsForCell_[*cell].push_back(segment);
        cellForSegment_.push_back(*cell);
        *out = segment;
      }
    }
  }
breznak commented 5 years ago

HI @Thanh-Binh , can you provide a small running test? I'll try to fix that in https://github.com/htm-community/nupic.cpp

Thanh-Binh commented 5 years ago

@breznak I will have an idea for running test... and provide it soon ...

Thanh-Binh commented 5 years ago

In the meantime after analysing the codes in SparseMatrix.hpp, I found out that the size of the input "overlap" for computeActivity() should be initialized by the current number of synapses for all segments, exactly by calling matrix.nRows(), because it will be changed after calling createSegments(), and growSynapsesForSample(). Now it works correctly, and the debug time helps me to understand better the nupic codes ... so that I can close this ... Thanks again to Mark....