Fix for issue #1339: simulating with 'times' conflicted with simulations without.

apps/rr-sbml-benchmark/main.cpp

@@ -123,21 +123,21 @@ int main(int argc, char** argv)
         {
             int steps = strtol(argv[6], NULL, 0);
             if (steps > 0) {
-                settings.steps = steps;
+                settings.setSteps(steps);
             }
         }
 
         if (argc > 7)
         {
             double dur = atof(argv[7]);
             if (dur > 0) {
-                settings.duration = dur;
+                settings.setDuration(dur);
             }
         }
 
 	    roadRunner.getIntegrator()->setValue("stiff", false);
 
-        std::cout << "running for " << settings.steps << ", duration " << settings.duration << std::endl;
+        std::cout << "running for " << settings.getSteps() << ", duration " << settings.getDuration() << std::endl;
 
         std::cout << "absolute: " << roadRunner.getIntegrator()->getValue("absolute_tolerance").convert<bool>() << std::endl;
 	    std::cout << "relative: " << roadRunner.getIntegrator()->getValue("relative_tolerance").convert<bool>() << std::endl;

apps/rr/main.cpp

@@ -71,9 +71,9 @@ int main(int argc, char * argv[])
         RoadRunner rr(args.ModelFileName);
 
         SimulateOptions& opt = rr.getSimulateOptions();
-        opt.start = args.StartTime;
-        opt.duration = args.EndTime - args.StartTime;
-        opt.steps = args.Steps;
+        opt.setStartTime(args.StartTime);
+        opt.setDuration(args.EndTime - args.StartTime);
+        opt.setSteps(args.Steps);
         if(args.variableStep) 
 		{
 			rr.getIntegrator()->setValue("variable_step_size", true);

cmake/AddTestExecutable.cmake

@@ -36,6 +36,7 @@ function(add_test_executable TEST_TARGET OUT_VARIABLE)
     set(TEST_ENV_VARS "testdir=${RR_ROOT}/test" "CTEST_OUTPUT_ON_FAILURE=TRUE")
     gtest_discover_tests(
             ${TEST_TARGET}
+            DISCOVERY_MODE PRE_TEST
             DISCOVERY_TIMEOUT 500
             PROPERTIES
             TIMEOUT 500

rrplugins/examples/cpp/one_rr/main.cpp

@@ -33,9 +33,9 @@ int main(int argc, char** argv)
         rr1.load(modelFile, &opt);
 
         SimulateOptions simOpt;
-        simOpt.start = 0;
-        simOpt.start = 10;
-        simOpt.steps= 511;
+        simOpt.setStartTime(0);
+        simOpt.setStartTime(10);
+        simOpt.setSteps(511);
 
         rr1.setGlobalParameterByIndex(2, 0.2);
         double ss = rr1.steadyState();

rrplugins/plugins/nelder_mead/lib/rr_example/sbml_model_one_para.cpp

@@ -60,9 +60,9 @@ double objfun(double par[], const void* userData)
     }
 
     rr::SimulateOptions opt;
-    opt.start       = expData.getTimeStart();
-    opt.duration    = expData.getTimeEnd() - expData.getTimeStart();
-    opt.steps       = expData.rSize() -1;
+    opt.setStartTime(expData.getTimeStart());
+    opt.setDuration(expData.getTimeEnd() - expData.getTimeStart());
+    opt.setSteps(expData.rSize() -1);
     TelluriumData simData(rr.simulate(&opt));
 
     //Calculate Chi Square using the ChiSquare plugin

source/rrRoadRunner.cpp

@@ -1574,16 +1574,12 @@ namespace rr {
             throw CoreException(gEmptyModelMessage);
         }
 
-        double start = impl->simulateOpt.start;
-        double duration = impl->simulateOpt.duration;
-        int steps = impl->simulateOpt.steps;
-        impl->simulateOpt.start = 0;
-        impl->simulateOpt.duration = 10.0;
-        impl->simulateOpt.steps = 100;
+        SimulateOptions savedOpt = impl->simulateOpt;
+        impl->simulateOpt.setStartTime(0);
+        impl->simulateOpt.setDuration(10.0);
+        impl->simulateOpt.setSteps(100);
         simulate();
-        impl->simulateOpt.start = start;
-        impl->simulateOpt.duration = duration;
-        impl->simulateOpt.steps = steps;
+        impl->simulateOpt = savedOpt;
 
         return steadyState();
     }
@@ -2370,19 +2366,20 @@ namespace rr {
     }
 
     const ls::DoubleMatrix *RoadRunner::simulate(double start, double stop, int points) {
-        SimulateOptions opt;
-        opt.start = start;
-        opt.duration = stop;
+        // Use existing simulateOpt, so we don't lose other options in it.
+        get_self();
+        self.simulateOpt.setStartTime(start);
+        self.simulateOpt.setDuration(stop);
         // substract 1 so that start, stop, num has the same meaning as it does in numpy functions
         // i.e. see https://numpy.org/doc/stable/reference/generated/numpy.linspace.html
-        opt.steps = points - 1;
-        return simulate(&opt);
+        self.simulateOpt.setSteps(points - 1);
+        return simulate();
     }
 
     const ls::DoubleMatrix *RoadRunner::simulate(const std::vector<double> &times) {
-        SimulateOptions opt;
-        opt.times = times;
-        return simulate(&opt);
+        get_self();
+        self.simulateOpt.setTimes(times);
+        return simulate();
     }
 
     Matrix3D<double, double> RoadRunner::timeSeriesSensitivities(

source/rrRoadRunner.h

@@ -330,9 +330,9 @@ namespace rr {
          * @code
          * RoadRunner rr = RoadRunner("someFile.xml");
          * SimulateOptions opt = rr.getSimulateOptions();
-         * opt.start = 0;
-         * opt.duration = 10;
-         * opt.steps = 1000;
+         * opt.setStartTime(0);
+         * opt.setDuration(10);
+         * opt.setSteps(1000);
          * const DoubleMatrix *result = rr.simulate(&opt);
          * @endcode
          *
@@ -343,9 +343,9 @@ namespace rr {
          * RoadRunner rr = RoadRunner("someFile.xml");
          * rr.setIntegrator("gillespie");
          * SimulateOptions opt;
-         * opt.start = 0;
-         * opt.duration = 10;
-         * opt.steps = 1000;
+         * opt.setStartTime(0);
+         * opt.setDuration(10);
+         * opt.setSteps(1000);
          * opt.setItem("stiff", true);
          * opt.setItem("seed", 12345);
          * const DoubleMatrix *result = rr.simulate(&opt);

source/rrRoadRunnerOptions.cpp

@@ -72,6 +72,56 @@ namespace rr {
         hstep = 0;
     }
 
+    void SimulateOptions::setTimes(const std::vector<double>& newTimes)
+    {
+        times = newTimes;
+        // 'times' is now authoritative: discard whatever steps-based state a
+        // previous call may have left behind, so it can't be mistaken for a
+        // deliberate, still-current steps request.
+        steps = Config::getInt(Config::SIMULATEOPTIONS_STEPS);
+        start = 0;
+        duration = Config::getDouble(Config::SIMULATEOPTIONS_DURATION);
+        hstep = 0;
+    }
+
+    const std::vector<double>& SimulateOptions::getTimes() const
+    {
+        return times;
+    }
+
+    void SimulateOptions::setStartTime(double newStart)
+    {
+        times.clear();
+        start = newStart;
+    }
+
+    double SimulateOptions::getStartTime() const
+    {
+        return start;
+    }
+
+    void SimulateOptions::setDuration(double newDuration)
+    {
+        times.clear();
+        duration = newDuration;
+    }
+
+    double SimulateOptions::getDuration() const
+    {
+        return duration;
+    }
+
+    void SimulateOptions::setSteps(int newSteps)
+    {
+        times.clear();
+        steps = newSteps;
+    }
+
+    int SimulateOptions::getSteps() const
+    {
+        return steps;
+    }
+
     void SimulateOptions::loadSBMLSettings(const std::string &fname) {
         if (!fname.size()) {
             rrLog(Logger::LOG_ERROR) << "Empty file name for setings file";

source/rrRoadRunnerOptions.h

@@ -314,26 +314,6 @@ namespace rr
         */
         bool copy_result;
 
-        /**
-        * The number of steps at which the output is sampled. The samples are evenly spaced.
-        * When a simulation system calculates the data points to record, it will typically
-        * divide the duration by the number of time steps. Thus, for X steps, the output
-        * will have X+1 data rows.
-        */
-        int steps;
-
-        /**
-        * The start time of the simulation time-series data.
-        * Often this is 0, but not necessarily.
-        */
-        double start;
-
-        /**
-        * The duration of the simulation run, in the model's units of time.
-        */
-        double duration;
-
-
         /**
         * The ouptut file for simulation results. If non-empty, then the
         * simulation results are batch-written to output_file every
@@ -372,11 +352,45 @@ namespace rr
         */
         std::vector<std::string> concentrations;
 
-        /*
-        * A list of the requested output times.  If set, the simulator will only
-        * report simulation output at the requested values.
+        /**
+        * Set the requested output times.  The simulation will report output only
+        * at these values.  'times' takes precedence over 'steps'/'start'/'duration':
+        * setting it clears any previously-set steps-based state so that
+        * times-mode and steps-mode are always freely interleavable regardless of
+        * what a previous call configured.
         */
-        std::vector<double> times;
+        void setTimes(const std::vector<double>& times);
+
+        /**
+        * The requested output times, or empty if the simulation is steps-based.
+        */
+        const std::vector<double>& getTimes() const;
+
+        /**
+        * Set the start time of the simulation time-series data.  Often this is 0,
+        * but not necessarily.  Clears any previously-set 'times', since 'times'
+        * and the steps/start/duration triplet are mutually exclusive.
+        */
+        void setStartTime(double start);
+
+        double getStartTime() const;
+
+        /**
+        * Set the duration of the simulation run, in the model's units of time.
+        * Clears any previously-set 'times'.
+        */
+        void setDuration(double duration);
+
+        double getDuration() const;
+
+        /**
+        * Set the number of steps at which the output is sampled. The samples are
+        * evenly spaced. Thus, for X steps, the output will have X+1 data rows.
+        * Clears any previously-set 'times'.
+        */
+        void setSteps(int steps);
+
+        int getSteps() const;
 
         /**
         * get a description of this object, compatable with python __str__
@@ -409,6 +423,35 @@ namespace rr
 
     private:
 
+        // RoadRunner is a tightly-coupled internal collaborator that needs
+        // direct access to these fields.
+        friend class RoadRunner;
+
+        /**
+        * The number of steps at which the output is sampled. The samples are evenly spaced.
+        * When a simulation system calculates the data points to record, it will typically
+        * divide the duration by the number of time steps. Thus, for X steps, the output
+        * will have X+1 data rows.
+        */
+        int steps;
+
+        /**
+        * The start time of the simulation time-series data.
+        * Often this is 0, but not necessarily.
+        */
+        double start;
+
+        /**
+        * The duration of the simulation run, in the model's units of time.
+        */
+        double duration;
+
+        /**
+        * The requested output times.  If set, the simulator will only report
+        * simulation output at the requested values.
+        */
+        std::vector<double> times;
+
         double hstep; //Used if we have a duration and number of steps, but not 'times'.
 
 

source/rrSBMLModelSimulation.cpp

@@ -208,19 +208,19 @@ namespace rr
 
     bool SBMLModelSimulation::SetTimeStart(const double& startTime)
     {
-        mSettings.start = startTime;
+        mSettings.setStartTime(startTime);
         return true;
     }
 
     bool SBMLModelSimulation::SetTimeEnd(const double& endTime)
     {
-        mSettings.duration = endTime - mSettings.start;
+        mSettings.setDuration(endTime - mSettings.getStartTime());
         return true;
     }
 
     bool SBMLModelSimulation::SetNumberOfPoints(const int& steps)
     {
-        mSettings.steps = steps;
+        mSettings.setSteps(steps);
         return true;
     }
 

test/c_api_core/CAPICoreTest.cpp

@@ -260,8 +260,8 @@ TEST_F(CAPICoreTest, CheckRK4WorksFromC) {
     }
 
     SimulateOptions opt;
-    opt.start = 0;
-    opt.duration = 10;
+    opt.setStartTime(0);
+    opt.setDuration(10);
     auto *cvode = rr.simulate(&opt);
 
     rr.setIntegrator("rk4");

test/c_api_core/CMakeLists.txt

@@ -5,6 +5,7 @@ add_test_executable(test_c_api_core test_targets
         CAPICoreTest
         libstruct.cpp
         model_editing.cpp
+        SimulateOptionsCAPITests.cpp
         )