last few changes for cran re-submit

CRAN-SUBMISSION

@@ -1,3 +1,3 @@
-Version: 0.1.0
-Date: 2023-10-13 20:40:05 UTC
-SHA: 62f71c683b4ca6ec3f793b06af65f75d08bab1c1
+Version: 0.1.1
+Date: 2023-10-21 18:36:16 UTC
+SHA: 7df5f537bb30d6c0ebdf94d1720540e89f94f9f7

R/orsf_pd.R

@@ -526,19 +526,26 @@ orsf_pred_dependence <- function(object,
 
   pd_vals[[i]] <- rbindlist(pd_vals[[i]], idcol = 'id_variable')
 
-  pd_vals[[i]] <- merge(pd_vals[[i]],
-                        as.data.table(pd_bind[[i]]),
+  # this seems awkward but the reason I convert back to data.frame
+  # here is to avoid a potential memory leak from forder & bmerge.
+  # I have no idea why this memory leak may be occurring but it does
+  # not if I apply merge.data.frame instead of merge.data.table
+  pd_vals[[i]] <- merge(as.data.frame(pd_vals[[i]]),
+                        as.data.frame(pd_bind[[i]]),
                         by = 'id_variable')
 
  }
 
-
  out <- rbindlist(pd_vals)
 
- # missings may occur when oobag=TRUE and n_tree is small
- if(type_output == 'ice') out <- collapse::na_omit(out, cols = 'pred')
+ # # missings may occur when oobag=TRUE and n_tree is small
+ # if(type_output == 'ice') {
+ #  out <- collapse::na_omit(out, cols = 'pred')
+ # }
+
+ ids <- c('id_variable')
 
- ids <- c('id_variable', if(type_output == 'ice') 'id_row')
+ if(type_output == 'ice') ids <- c(ids, 'id_row')
 
  mid <- setdiff(names(out), c(ids, 'mean', prob_labels, 'pred'))
 

cran-comments.md

@@ -1,6 +1,8 @@
 ## Version 0.1.1
 
-This version is being submitted to CRAN early due to a memory error that was identified in version 1.0.0. I apologize for the oversight. As `aorsf` would be removed from CRAN if the issue is not fixed before October 28, I would like to request an expedited submission. I have run the current submission's tests and examples with valgrind to ensure the memory error has been fixed. 
+Update, October 21: I have updated the submission to fix memory leaks. Many of the leaks were caused by my omission of a virtual de-constructor for derived classes or by omission of a delete statement for dynamically allocated memory. I apologize for these oversights. After reviewing, you may still see a possible memory leak from `orsf_ice` functions. From what I can tell, this possible leak could either be measurement error or could be attributed to `data.table`. I do not think it's from `aorsf`.
+
+Initial submission: This version is being submitted to CRAN early due to a memory error that was identified in version 1.0.0. I apologize for the oversight. As `aorsf` would be removed from CRAN if the issue is not fixed before October 28, I would like to request an expedited submission. I have run the current submission's tests and examples with valgrind to ensure the memory error has been fixed. 
 
 ## R CMD check results
 

src/Forest.cpp

@@ -532,6 +532,8 @@ std::vector<std::vector<arma::mat>> Forest::compute_dependence(bool oobag){
 
     result_k.push_back(preds);
 
+
+
    }
 
   }

tests/testthat/test-orsf_pd.R

@@ -16,29 +16,29 @@ test_that(
 
 fit <- fit_standard_pbc$fast
 
-# test_that(
-#  "user cant supply empty pred_spec",
-#  code = {
-#   expect_error(
-#    orsf_ice_oob(fit, pred_spec = list()),
-#    regexp = 'pred_spec is empty'
-#   )
-#  }
-# )
+test_that(
+ "user cant supply empty pred_spec",
+ code = {
+  expect_error(
+   orsf_ice_oob(fit, pred_spec = list()),
+   regexp = 'pred_spec is empty'
+  )
+ }
+)
 
-# test_that(
-#  "user cant supply pred_spec with non-matching names",
-#  code = {
-#   expect_error(
-#    orsf_ice_oob(fit,
-#                 pred_spec = list(bili = 1:5,
-#                                  nope = c(1,2),
-#                                  no_sir = 1),
-#                 pred_horizon = 1000),
-#    regexp = 'nope and no_sir'
-#   )
-#  }
-# )
+test_that(
+ "user cant supply pred_spec with non-matching names",
+ code = {
+  expect_error(
+   orsf_ice_oob(fit,
+                pred_spec = list(bili = 1:5,
+                                 nope = c(1,2),
+                                 no_sir = 1),
+                pred_horizon = 1000),
+   regexp = 'nope and no_sir'
+  )
+ }
+)
 
 bad_value_lower <- quantile(pbc_orsf$bili, probs = 0.01)
 bad_value_upper <- quantile(pbc_orsf$bili, probs = 0.99)
@@ -57,9 +57,9 @@ test_that(
 )
 
 funs <- list(
- # ice_new = orsf_ice_new,
- # ice_inb = orsf_ice_inb,
- # ice_oob = orsf_ice_oob,
+ ice_new = orsf_ice_new,
+ ice_inb = orsf_ice_inb,
+ ice_oob = orsf_ice_oob,
  pd_new = orsf_pd_new,
  pd_inb = orsf_pd_inb,
  pd_oob = orsf_pd_oob
@@ -147,12 +147,12 @@ for(i in seq_along(funs)){
 }
 
 
-# pd_vals_ice <- orsf_ice_new(
-#  fit,
-#  new_data = pbc_orsf,
-#  pred_spec = list(bili = 1:4),
-#  pred_horizon = 1000
-# )
+pd_vals_ice <- orsf_ice_new(
+ fit,
+ new_data = pbc_orsf,
+ pred_spec = list(bili = 1:4),
+ pred_horizon = 1000
+)
 
 pd_vals_smry <- orsf_pd_new(
  fit,
@@ -161,19 +161,19 @@ pd_vals_smry <- orsf_pd_new(
  pred_horizon = 1000
 )
 
-# test_that(
-#  'ice values summarized are the same as pd values',
-#  code = {
-#
-#   pd_vals_check <- pd_vals_ice[, .(medn = median(pred)), by = id_variable]
-#
-#   expect_equal(
-#    pd_vals_check$medn,
-#    pd_vals_smry$medn
-#   )
-#
-#  }
-# )
+test_that(
+ 'ice values summarized are the same as pd values',
+ code = {
+
+  pd_vals_check <- pd_vals_ice[, .(medn = median(pred)), by = id_variable]
+
+  expect_equal(
+   pd_vals_check$medn,
+   pd_vals_smry$medn
+  )
+
+ }
+)
 
 
 test_that(
@@ -207,15 +207,15 @@ test_that(
   expect_lte(pd_smry_multi_horiz$medn[1], pd_smry_multi_horiz$medn[2])
   expect_lte(pd_smry_multi_horiz$medn[2], pd_smry_multi_horiz$medn[3])
 
-  # pd_ice_multi_horiz <- orsf_ice_oob(
-  #  fit,
-  #  pred_spec = list(bili = 1),
-  #  pred_horizon = c(1000, 2000, 3000)
-  # )
-  #
-  # ice_check <- pd_ice_multi_horiz[, .(m = mean(pred)), by = pred_horizon]
-  #
-  # expect_equal(ice_check$m, pd_smry_multi_horiz$mean)
+  pd_ice_multi_horiz <- orsf_ice_oob(
+   fit,
+   pred_spec = list(bili = 1),
+   pred_horizon = c(1000, 2000, 3000)
+  )
+
+  ice_check <- pd_ice_multi_horiz[, .(m = mean(pred, na.rm=TRUE)), by = pred_horizon]
+
+  expect_equal(ice_check$m, pd_smry_multi_horiz$mean)
 
  }