fix sources to properly restore distributions and trajectories, closes #324

Author: Enchufa2

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: simmer
 Type: Package
 Title: Discrete-Event Simulation for R
-Version: 4.4.6.5
+Version: 4.4.6.6
 Authors@R: c(
     person("Iñaki", "Ucar", email="iucar@fedoraproject.org",
       role=c("aut", "cph", "cre"), comment=c(ORCID="0000-0001-6403-5550")),

NEWS.md

@@ -1,6 +1,7 @@
 # simmer devel
 
 - Fix `set_source()` to avoid leaking arrivals from the old source (#322).
+- Fix sources to properly restore distributions and trajectories (#324).
 
 # simmer 4.4.6.4
 

inst/include/simmer/process/arrival.h

@@ -1,5 +1,5 @@
 // Copyright (C) 2015-2016 Bart Smeets and Iñaki Ucar
-// Copyright (C) 2016-2023 Iñaki Ucar
+// Copyright (C) 2016-2024 Iñaki Ucar
 //
 // This file is part of simmer.
 //
@@ -58,16 +58,16 @@ namespace simmer {
 
     /**
     * Constructor.
-    * @param sim             a pointer to the simulator
-    * @param name            the name
-    * @param mon             int that indicates whether this entity must be monitored
-    * @param order           priority, preemptible, restart
-    * @param first_activity  the first activity of a user-defined R trajectory
+    * @param sim    a pointer to the simulator
+    * @param name   the name
+    * @param mon    int that indicates whether this entity must be monitored
+    * @param order  priority, preemptible, restart
+    * @param head   the first activity of a user-defined R trajectory
     */
     Arrival(Simulator* sim, const std::string& name, int mon, Order order,
-            Activity* first_activity, int priority = 0, Source* src = NULL)
+            Activity* head, int priority = 0, Source* src = NULL)
       : Process(sim, name, mon, priority), order(order), src(src), paused(0),
-        sync(new Arrival*(NULL)), clones(new int(0)), activity(first_activity),
+        sync(new Arrival*(NULL)), clones(new int(0)), activity(head),
         timer(NULL), dropout(NULL), batch(NULL), act_shd(new ActVec())
     { init(); }
 

inst/include/simmer/process/datasrc.h

@@ -30,37 +30,46 @@ namespace simmer {
             const REnv& trj, RData data, int batch, const std::string& col_time,
             const VEC<std::string>& col_attrs, const OPT<std::string>& col_priority,
             const OPT<std::string>& col_preemptible, const OPT<std::string>& col_restart)
-      : Source(sim, name_prefix, mon, trj, Order()), source(data), batch(batch),
+      : Source(sim, name_prefix, mon, trj, Order()), source_(data), batch(batch),
         col_time(col_time), col_attrs(col_attrs), col_priority(col_priority),
-        col_preemptible(col_preemptible), col_restart(col_restart) { set_source_impl(data); }
+        col_preemptible(col_preemptible), col_restart(col_restart) { reset(); }
+
+    void reset() {
+      Source::reset();
+      source = source_;
+      set_source_impl(source);
+    }
 
     void run() {
       double delay = 0;
       int i = 0;
 
       while (i++ != batch) {
-        if (time.size() <= count || check_stop(time[count]))
+        if (time.size() <= index || check_stop(time[index]))
           return;
-        delay += time[count];
+        delay += time[index];
 
         Arrival* arrival = new_arrival(delay);
 
         for (size_t j = 0; j < col_attrs.size(); ++j)
-          arrival->set_attribute(col_attrs[j], attrs[j][count-1]);
+          arrival->set_attribute(col_attrs[j], attrs[j][index]);
 
         if (col_priority)
-          arrival->order.set_priority(priority[count-1]);
+          arrival->order.set_priority(priority[index]);
         if (col_preemptible)
-          arrival->order.set_preemptible(preemptible[count-1]);
+          arrival->order.set_preemptible(preemptible[index]);
         if (col_restart)
-          arrival->order.set_restart(restart[count-1]);
+          arrival->order.set_restart(restart[index]);
+
+        index++;
       }
       // schedule the generator
       sim->schedule(delay, this, Source::priority);
     }
 
   private:
-    RData source;
+    int index;
+    RData source_, source;
     int batch;
     std::string col_time;
     VEC<std::string> col_attrs;
@@ -91,6 +100,7 @@ namespace simmer {
       if (col_restart && !df.containsElementNamed((*col_restart).c_str()))
         Rcpp::stop("column '%s' not present", *col_restart);
 
+      index = 0;
       source = df;
       time = source[col_time];
       attrs.clear();

inst/include/simmer/process/generator.h

@@ -31,10 +31,12 @@ namespace simmer {
   public:
     Generator(Simulator* sim, const std::string& name_prefix, int mon,
               const REnv& trj, const RFn& dist, const Order& order)
-      : Source(sim, name_prefix, mon, trj, order), source(dist) {}
+      : Source(sim, name_prefix, mon, trj, order), source_(dist), source(dist)
+        { reset(); }
 
     void reset() {
       Source::reset();
+      source = source_;
       RFn reset_fun(source.attr("reset"));
       reset_fun();
     }
@@ -57,7 +59,7 @@ namespace simmer {
     }
 
   private:
-    RFn source;
+    RFn source_, source;
 
     void set_source_impl(const std::any& new_source) {
       if (new_source.type() != typeid(RFn))

inst/include/simmer/process/source.h

@@ -63,11 +63,12 @@ namespace simmer {
     */
     Source(Simulator* sim, const std::string& name_prefix, int mon,
            const REnv& trj, const Order& order)
-      : Process(sim, name_prefix, mon, PRIORITY_MIN), count(0), order(order),
-        first_activity(internal::head(trj)), trj(trj) {}
+      : Process(sim, name_prefix, mon, PRIORITY_MIN), order(order), trj_(trj) {}
 
     virtual void reset() {
       count = 0;
+      trj = trj_;
+      head = internal::head(trj);
       ahead.clear();
     }
 
@@ -81,7 +82,7 @@ namespace simmer {
       return Process::deactivate();
     }
 
-    int get_n_generated() const { return count; }
+    int get_count() const { return count; }
 
     REnv get_trajectory() const { return trj; }
 
@@ -93,27 +94,23 @@ namespace simmer {
 
     void set_trajectory(const REnv& new_trj) {
       trj = new_trj;
-      first_activity = internal::head(trj);
+      head = internal::head(trj);
     }
 
   protected:
-    int count;                /**< number of arrivals generated */
-    Order order;
-    Activity* first_activity;
-
     Arrival* new_arrival(double delay) {
       // format the name and create the next arrival
       std::string arr_name = MakeString() << name << count++;
       Arrival* arrival = new Arrival(
-        sim, arr_name, is_monitored(), order, first_activity, count, this);
+        sim, arr_name, is_monitored(), order, head, count, this);
       ahead.emplace(arrival);
 
       if (sim->verbose) sim->print("source", name, "new", arr_name,
           MakeString() << (sim->now() + delay));
 
       // schedule the arrival
-      sim->schedule(delay, arrival, first_activity && first_activity->priority ?
-                      first_activity->priority : count);
+      sim->schedule(delay, arrival, head && head->priority ?
+                      head->priority : count);
 
       return arrival;
     }
@@ -129,7 +126,10 @@ namespace simmer {
     }
 
   private:
-    REnv trj;
+    int count;                /**< number of arrivals generated */
+    Order order;
+    Activity* head;
+    REnv trj_, trj;
     ArrSet ahead;
 
     virtual void set_source_impl(const std::any& new_source) = 0;

src/process.cpp

@@ -1,6 +1,6 @@
 // Copyright (C) 2014 Bart Smeets
 // Copyright (C) 2015 Iñaki Ucar and Bart Smeets
-// Copyright (C) 2015-2023 Iñaki Ucar
+// Copyright (C) 2015-2024 Iñaki Ucar
 //
 // This file is part of simmer.
 //
@@ -35,7 +35,7 @@ Vector<RTYPE> get_param(SEXP sim_, const VEC<std::string>& names,
 
 //[[Rcpp::export]]
 SEXP get_n_generated_(SEXP sim_, const std::vector<std::string>& names) {
-  return get_param<INTSXP,int>(sim_, names, std::mem_fn(&Source::get_n_generated));
+  return get_param<INTSXP,int>(sim_, names, std::mem_fn(&Source::get_count));
 }
 
 //[[Rcpp::export]]

tests/testthat/test-simmer-dataframe.R

@@ -1,4 +1,4 @@
-# Copyright (C) 2018-2023 Iñaki Ucar
+# Copyright (C) 2018-2024 Iñaki Ucar
 #
 # This file is part of simmer.
 #
@@ -93,13 +93,28 @@ test_that("generates the expected amount", {
 })
 
 test_that("data sources are reset", {
-  DF <- data.frame(time=rep(1, 3))
-
   expect_equal(3, simmer(verbose = env_verbose) %>%
-    add_dataframe("dummy", trajectory(), DF) %>%
+    add_dataframe("dummy", trajectory(), data.frame(time=rep(1, 3))) %>%
     run() %>% reset() %>% run() %>%
     get_mon_arrivals() %>% nrow()
   )
+
+  t <- trajectory() %>%
+    set_source("dummy", data.frame(time=10)) %>%
+    set_trajectory("dummy", trajectory() %>% timeout(1))
+
+  env <- simmer(verbose = env_verbose) %>%
+    add_dataframe("dummy", t, data.frame(time=0))
+
+  df1 <- env %>%
+    run() %>%
+    get_mon_arrivals()
+  df2 <- env %>%
+    reset() %>%
+    run() %>%
+    get_mon_arrivals()
+
+  expect_equal(df1, df2)
 })
 
 test_that("priorities are set", {

tests/testthat/test-simmer-generator.R

@@ -1,5 +1,5 @@
 # Copyright (C) 2015 Iñaki Ucar and Bart Smeets
-# Copyright (C) 2015-2023 Iñaki Ucar
+# Copyright (C) 2015-2024 Iñaki Ucar
 #
 # This file is part of simmer.
 #
@@ -58,6 +58,23 @@ test_that("generators are reset", {
     run() %>% reset() %>% run() %>%
     get_mon_arrivals() %>% nrow()
   )
+
+  t <- trajectory() %>%
+    set_source("dummy", at(10)) %>%
+    set_trajectory("dummy", trajectory() %>% timeout(1))
+
+  env <- simmer(verbose = env_verbose) %>%
+    add_generator("dummy", t, at(0))
+
+  df1 <- env %>%
+    run() %>%
+    get_mon_arrivals()
+  df2 <- env %>%
+    reset() %>%
+    run() %>%
+    get_mon_arrivals()
+
+  expect_equal(df1, df2)
 })
 
 test_that("preemptible < priority shows a warning", {