Fixes regression caused by #387

.ci/travis_build_linux.sh

@@ -9,6 +9,9 @@
 set -e
 set -x
 
+BUILDDIR=_build_travis
+mkdir -p $BUILDDIR
+
 PYTHON2="/usr/bin/python2"
 PYTHON3="/usr/bin/python3"
 
@@ -32,32 +35,36 @@ $PYTHON3 -m compileall -q .
 
 # Python3 with GSL in debug more.
 (
-    mkdir -p _GSL_BUILD_PY3 && cd _GSL_BUILD_PY3 && \
+    mkdir -p $BUILDDIR && cd $BUILDDIR && \
         cmake -DPYTHON_EXECUTABLE=$PYTHON3 \
         -DCMAKE_INSTALL_PREFIX=/usr -DDEBUG=ON ..
-    # Don't run test_long prefix. They take very long time in DEBUG mode.
-    $MAKE && MOOSE_NUM_THREADS=$NPROC ctest -j$NPROC --output-on-failure -E ".*test_long*"
+    $MAKE
+    # Run with valgrind to log any memory leak.
+    valgrind --leak-check=full ./moose.bin -q -u 
+
+    # Run all tests in debug mode.
+    MOOSE_NUM_THREADS=$NPROC ctest -j$NPROC --output-on-failure 
     make install || sudo make install 
     cd /tmp
     $PYTHON3 -c 'import moose;print(moose.__file__);print(moose.version())'
 )
 
-# BOOST and python3
-(
-    mkdir -p _BOOST_BUILD_PY3 && cd _BOOST_BUILD_PY3 && \
-        cmake -DWITH_BOOST_ODE=ON -DPYTHON_EXECUTABLE="$PYTHON3" \
-        -DCMAKE_INSTALL_PREFIX=/usr ..
-    $MAKE && MOOSE_NUM_THREADS=$NPROC ctest -j$NPROC --output-on-failure 
-)
-
 # GSL and python2, failure is allowed
 set +e
 (
-    BUILDDIR=_GSL_PY2
     mkdir -p $BUILDDIR && cd $BUILDDIR && \
         cmake -DPYTHON_EXECUTABLE=$PYTHON2 -DCMAKE_INSTALL_PREFIX=/usr ..
     $MAKE && MOOSE_NUM_THREADS=$NPROC ctest -j$NPROC --output-on-failure
 )
 set -e
 
+
+# BOOST and python3
+(
+    mkdir -p $BUILDDIR && cd $BUILDDIR && \
+        cmake -DWITH_BOOST_ODE=ON -DPYTHON_EXECUTABLE="$PYTHON3" \
+        -DCMAKE_INSTALL_PREFIX=/usr ..
+    $MAKE && MOOSE_NUM_THREADS=$NPROC ctest -j$NPROC --output-on-failure 
+)
+
 echo "All done"

.ci/travis_build_osx.sh

@@ -20,6 +20,8 @@
 set -o nounset                              # Treat unset variables as an error
 set -e
 
+BUILDDIR=_build_travis
+
 NPROC=$(nproc)
 (
     # Make sure not to pick up python from /opt.
@@ -32,14 +34,14 @@ NPROC=$(nproc)
     $PYTHON3 -m pip install python-libsbml --user
     $PYTHON3 -m pip install pyneuroml --user
 
-    mkdir -p _GSL_BUILD && cd _GSL_BUILD \
+    mkdir -p $BUILDDIR && cd $BUILDDIR \
         && cmake -DPYTHON_EXECUTABLE=$PYTHON3 \
         ..
     make pylint -j$NPROC
     make -j$NPROC && MOOSE_NUM_THREAD=$NPROC ctest --output-on-failure -j$NPROC
 
     cd .. # Now with boost.
-    mkdir -p _BOOST_BUILD && cd _BOOST_BUILD \
+    mkdir -p $BUILDDIR && cd $BUILDDIR \
         && cmake -DWITH_BOOST_ODE=ON \
         -DPYTHON_EXECUTABLE=`which python3` ..
 

.ci/travis_prepare_linux.sh

@@ -13,6 +13,7 @@ apt-get install -qq make cmake
 apt-get install -qq python-numpy python-matplotlib python-networkx python-pip
 apt-get install -qq python3-numpy python3-matplotlib python3-networkx python3-pip
 apt-get install -qq python-tk python3-tk
+apt-get install -qq valgrind
 
 # Gsl
 apt-get install -qq libgsl0-dev || apt-get install -qq libgsl-dev

.gitlab-ci.yml

@@ -13,21 +13,8 @@ build:
     - apt -y install python3-numpy python3-matplotlib python3-pip python3-dev
     - apt -y install python-numpy python-matplotlib python-pip python-dev
   script:
-    - python3 setup.py install
     - python3 -m pip install setuptools pip --user --upgrade
-    - python2 setup.py install 
-    - python2 -m pip install setuptools pip --user --upgrade
-    - python3 setup.py sdist
-  artifacts:
-    paths:
-      - mybinary
-      # depending on your build setup it's most likely a good idea to cache outputs to reduce the build time
-      # cache:
-      #   paths:
-      #     - "*.o"
-
-# run tests using the binary built before
-test:
-  stage: test
-  script:
-    - python3 setup.py test
+    - python3 -m pip install pyneuroml --user --upgrade
+    - python3 setup.py build test 
+    - python3 setup.py install --user
+    - python3 -c "import moose; moose.test()"

CMakeLists.txt

@@ -84,11 +84,7 @@ elseif(ENABLE_UNIT_TESTS)
 else()
     message(STATUS "Building for Release/No unit tests.")
     set(CMAKE_BUILD_TYPE Release)
-		add_definitions(-UDO_UNIT_TESTS -O3 -DDISABLE_DEBUG)
-
-    # DO NOT Treat all warnings as errors. With some compilers and newer versions
-    # this often causes headache.
-    # add_definitions(-Werror)
+    add_definitions(-UDO_UNIT_TESTS -O3 -DDISABLE_DEBUG)
 endif()
 
 if(GPROF AND "${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
@@ -160,20 +156,11 @@ if(WITH_GSL)
             "====================================================================\n"
             )
     endif(NOT GSL_FOUND)
-    add_definitions(-DUSE_GSL)
-    # GSL is also used in RNG (whenever applicable), therefore include paths are
-    # top level.
-    include_directories( ${GSL_INCLUDE_DIRS} )
 elseif(WITH_BOOST_ODE)
     find_package(Boost 1.53 REQUIRED)
     find_package(LAPACK REQUIRED)
 endif()
 
-# if boost ode is being used, don't use GSL.
-if(WITH_BOOST_ODE)
-    add_definitions(-DUSE_BOOST_ODE -UUSE_GSL)
-    include_directories(${Boost_INCLUDE_DIRS})
-endif()
 
 # Openmpi
 if(WITH_MPI)

builtins/Function.cpp

@@ -318,24 +318,8 @@ Function::Function():
 {
 }
 
-Function::Function(const Function& f) :
-    valid_(f.valid_),
-    numVar_(f.numVar_),
-    lastValue_(f.lastValue_),
-    rate_(f.rate_),
-    mode_(f.mode_),
-    useTrigger_(f.useTrigger_),
-    doEvalAtReinit_(f.doEvalAtReinit_),
-    t_(f.t_),
-    independent_(f.independent_),
-    xs_(f.xs_),
-    ys_(f.ys_),
-    stoich_(f.stoich_)
-{
-    parser_.LinkVariables(xs_, ys_, &t_);
-}
-
-// Careful: This is a critical function.
+// Careful: This is a critical function. Also since during zombiefication, deep
+// copy is expected. Merely copying the parser won't work.
 Function& Function::operator=(const Function& rhs)
 {
     // protect from self-assignment.
@@ -351,9 +335,23 @@ Function& Function::operator=(const Function& rhs)
     t_ = rhs.t_;
     rate_ = rhs.rate_;
     independent_ = rhs.independent_;
-    xs_ = rhs.xs_;
-    ys_ = rhs.ys_;
-    parser_.LinkVariables(xs_, ys_, &t_);
+
+    // Deep copy; create new Variable and constant to link with new parser.
+    // Zombification requires it. DO NOT just copy the object/pointer of
+    // MooseParser.
+    xs_.clear();
+    ys_.clear();
+    parser_.ClearAll();
+    if(rhs.parser_.GetExpr().size() > 0)
+    {
+        for(auto x: rhs.xs_)
+            xs_.push_back(shared_ptr<Variable>(new Variable()));
+        for(auto y: rhs.ys_)
+            ys_.push_back(shared_ptr<double>(new double(0.0)));
+
+        parser_.LinkVariables(xs_, ys_, &t_);
+        parser_.SetExpr(rhs.parser_.GetExpr());
+    }
     return *this;
 }
 
@@ -395,7 +393,7 @@ void Function::addVariable(const string& name)
         {
             // Equality with index because we cound from 0.
             for (size_t i = xs_.size(); i <= (size_t) index; i++)
-                xs_.push_back(new Variable());
+                xs_.push_back(shared_ptr<Variable>(new Variable()));
         }
 
         // This must be true.
@@ -414,9 +412,9 @@ void Function::addVariable(const string& name)
         {
             // Equality with index because we cound from 0.
             for (size_t i = ys_.size(); i <= (size_t) index; i++)
-                ys_.push_back(new double(0.0));
+                ys_.push_back(shared_ptr<double>(new double(0.0)));
         }
-        parser_.DefineVar(name, ys_[index]);
+        parser_.DefineVar(name, ys_[index].get());
     }
     else if (name == "t")
         parser_.DefineVar("t", &t_);
@@ -601,7 +599,7 @@ Variable * Function::getVar(unsigned int ii)
 {
     static Variable dummy;
     if ( ii < xs_.size())
-        return xs_[ii];
+        return xs_[ii].get();
 
     MOOSE_WARN( "Warning: Function::getVar: index: "
                 << ii << " is out of range: "

builtins/Function.h

@@ -24,7 +24,6 @@ class Function
 public:
     static const int VARMAX;
     Function();
-    Function(const Function& f);
 
     // Destructor.
     ~Function();
@@ -106,17 +105,15 @@ class Function
 
     // this stores variables received via incoming messages, identifiers of
     // the form x{i} are included in this
-    vector<Variable*> xs_;
+    vector<shared_ptr<Variable>> xs_;
 
     // this stores variable values pulled by sending request. identifiers of
     // the form y{i} are included in this
-    vector<double*> ys_;
+    vector<shared_ptr<double>> ys_;
 
     // Used by kinetic solvers when this is zombified.
     void* stoich_;
 
-    // Parser which should never be copied. Multithreaded programs may behave
-    // strangely if copy-constructor or operator()= is implemented.
     moose::MooseParser parser_;
 
 };

builtins/MooseParser.cpp

@@ -23,9 +23,7 @@ using namespace std;
 namespace moose
 {
 
-MooseParser::MooseParser() : 
-    expression_(new  moose::Parser::expression_t())
-    , symbol_tables_registered_(false)
+MooseParser::MooseParser()
 {
     Parser::symbol_table_t symbol_table;
 
@@ -39,8 +37,7 @@ MooseParser::MooseParser() :
     symbol_table.add_function( "srand2", MooseParser::SRand2 );
     symbol_table.add_function( "fmod", MooseParser::Fmod );
 
-    expression_->register_symbol_table(symbol_table);
-
+    expression_.register_symbol_table(symbol_table);
 }
 
 MooseParser::~MooseParser()
@@ -88,9 +85,14 @@ double MooseParser::Fmod( double a, double b )
 /*-----------------------------------------------------------------------------
  *  Get/Set
  *-----------------------------------------------------------------------------*/
-Parser::symbol_table_t& MooseParser::GetSymbolTable( ) const
+Parser::symbol_table_t& MooseParser::GetSymbolTable()
 {
-    return expression_->get_symbol_table();
+    return expression_.get_symbol_table();
+}
+
+const Parser::symbol_table_t& MooseParser::GetSymbolTable() const
+{
+    return expression_.get_symbol_table();
 }
 
 double MooseParser::GetVarValue(const string& name) const
@@ -210,7 +212,7 @@ bool MooseParser::CompileExpr()
     ASSERT_FALSE(expr_.empty(), __func__ << ": Empty expression not allowed here");
 
     Parser::parser_t  parser;
-    auto res = parser.compile(expr_, *expression_);
+    auto res = parser.compile(expr_, expression_);
     if(! res)
     {
         std::stringstream ss;
@@ -239,14 +241,14 @@ bool MooseParser::CompileExpr()
 
 double MooseParser::Derivative(const string& name) const
 {
-    return exprtk::derivative(*expression_, name);
+    return exprtk::derivative(expression_, name);
 }
 
 double MooseParser::Eval(bool check) const
 {
     if( expr_.empty())
         return 0.0;
-    double v = expression_->value();
+    double v = expression_.value();
     if(check)
     {
         if(! std::isfinite(v)) 
@@ -278,14 +280,19 @@ Parser::varmap_type MooseParser::GetConst( ) const
 
 void MooseParser::ClearVariables( )
 {
-    GetSymbolTable().clear_variables(false);
+    GetSymbolTable().clear_variables();
 }
 
 void MooseParser::ClearAll( )
 {
     ClearVariables();
 }
 
+void MooseParser::Reset( )
+{
+    expression_.release();
+}
+
 const string MooseParser::GetExpr( ) const
 {
     return expr_;
@@ -302,5 +309,17 @@ void MooseParser::LinkVariables(vector<Variable*>& xs, vector<double*>& ys, doub
     DefineVar("t", t);
 }
 
+void MooseParser::LinkVariables(vector<shared_ptr<Variable>>& xs, vector<shared_ptr<double>>& ys, double* t)
+{
+    for(size_t i = 0; i < xs.size(); i++)
+        DefineVar('x'+to_string(i), xs[i]->ref());
+
+    for (size_t i = 0; i < ys.size(); i++) 
+        DefineVar('y'+to_string(i), ys[i].get());
+
+    DefineVar("t", t);
+}
+
+
 
 } // namespace moose.