Skip to content

Commit

Permalink
possible leaks from data.table
Browse files Browse the repository at this point in the history
  • Loading branch information
bcjaeger committed Oct 25, 2023
1 parent d1c4c6d commit dfeb57d
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 9 deletions.
39 changes: 39 additions & 0 deletions R/melt_aorsf.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@

# need to make this to avoid possible memory leak in data.table melt

melt_aorsf <-
function(data,
id.vars,
measure.vars,
variable.name = "variable",
value.name = "value") {
if (!is.data.frame(data)) {
stop("Input 'data' must be a data frame.")
}

if (!is.character(id.vars)) {
stop("Input 'id.vars' must be a character vector.")
}

if (!is.character(measure.vars)) {
stop("Input 'measure.vars' must be a character vector.")
}

# Select id variables and measure variables
id_data <- data[id.vars]
measure_data <- data[measure.vars]

# Create a sequence variable to represent the variable names
variable_data <- rep(names(measure_data), each = nrow(data))

# Reshape the data
long_data <- data.frame(id_data,
variable = variable_data,
value = unlist(measure_data, use.names = FALSE))

names(long_data)[names(long_data) == 'variable'] <- variable.name
names(long_data)[names(long_data) == 'value'] <- value.name

return(long_data)
}

27 changes: 19 additions & 8 deletions R/orsf_pd.R
Original file line number Diff line number Diff line change
Expand Up @@ -606,14 +606,25 @@ orsf_pred_dependence <- function(object,
else
colnames(pd_vals[[i]][[j]]) <- c(paste(1:nrow(x_new)))

pd_vals[[i]][[j]] <- as.data.table(pd_vals[[i]][[j]],
keep.rownames = 'pred_horizon')

if(type_output == 'ice')
pd_vals[[i]][[j]] <- melt(data = pd_vals[[i]][[j]],
id.vars = 'pred_horizon',
variable.name = 'id_row',
value.name = 'pred')
ph <- rownames(pd_vals[[i]][[j]])

pd_vals[[i]][[j]] <- as.data.frame(pd_vals[[i]][[j]])

rownames(pd_vals[[i]][[j]]) <- NULL

pd_vals[[i]][[j]][['pred_horizon']] <- ph

if(type_output == 'ice'){

pd_vals[[i]][[j]] <- melt_aorsf(
data = pd_vals[[i]][[j]],
id.vars = 'pred_horizon',
variable.name = 'id_row',
value.name = 'pred',
measure.vars = setdiff(names(pd_vals[[i]][[j]]), 'pred_horizon'))

}


}

Expand Down
2 changes: 2 additions & 0 deletions cran-comments.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Version 0.1.1

Update, October 25: Thank you for helping me with this. I have tidied up threads and avoided usage of the `data.table` functions that were creating possible memory leaks. I have checked this submission locally with valgrind and on rhub, with both indicating 0 memory leaks. However, if this submission does not pass on your end, I would like to request an extension on the October 28th deadline.

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.
Expand Down
1 change: 0 additions & 1 deletion src/Makevars.win
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
CXX_STD = CXX14
PKG_CXXFLAGS = $(SHLIB_OPENMP_CXXFLAGS)
PKG_LIBS = $(SHLIB_OPENMP_CXXFLAGS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)

0 comments on commit dfeb57d

Please sign in to comment.