gbsg deprecation seems complete CRAN check passes

Author: larry-leon

NAMESPACE

@@ -113,6 +113,7 @@ export(run_single_consistency_split)
 export(save_forestplot)
 export(select_best_subgroup)
 export(sensitivity_analysis_k_inter)
+export(setup_gbsg_dgm)
 export(setup_parallel_SGcons)
 export(sg_consistency_out)
 export(sg_tables)

NEWS.md

@@ -0,0 +1,110 @@
+# forestsearch NEWS
+
+## forestsearch 0.2.0
+
+### New functions
+
+* `generate_aft_dgm_flex()` — general data-generating model (DGM) builder.
+  Replaces the GBSG-specific `create_gbsg_dgm()` as the primary DGM
+  constructor. Accepts any survival dataset and fits an accelerated failure
+  time (AFT) super-population model with user-specified treatment effect
+  heterogeneity parameters.
+
+* `simulate_from_dgm()` — general simulator for drawing trial replicates from
+  an `aft_dgm_flex` DGM. Replaces `simulate_from_gbsg_dgm()`. Column names
+  in the returned data frame use underscore notation (`y_sim`, `event_sim`,
+  `treat_sim`, `flag_harm`).
+
+* `run_simulation_analysis()` (general version) — simulation wrapper that
+  calls `simulate_from_dgm()` and accepts explicit column-name parameters,
+  making it applicable to any DGM built with `generate_aft_dgm_flex()`. The
+  GBSG dataset is now one application of this general pipeline rather than a
+  separate code path.
+
+* `setup_gbsg_dgm()` — convenience bridge function. Wraps
+  `create_gbsg_dgm()` (see Deprecated below) and reshapes its output to the
+  `aft_dgm_flex` class expected by `simulate_from_dgm()` and
+  `run_simulation_analysis()`. Existing GBSG-based simulation scripts can
+  adopt the general pipeline with a one-line change:
+  `dgm <- setup_gbsg_dgm(model = "alt", k_inter = k, seed = seed)`.
+
+### Deprecated functions
+
+The following functions are retained and fully functional but will be removed
+in a future version. Each emits a deprecation warning on first call with a
+concrete migration example.
+
+* `create_gbsg_dgm()` → use `generate_aft_dgm_flex()` or `setup_gbsg_dgm()`.
+
+* `simulate_from_gbsg_dgm()` → use `simulate_from_dgm(analysis_time = Inf)`.
+  Note: the new function defaults to `analysis_time = 48` (staggered-entry
+  administrative censoring); pass `analysis_time = Inf` to match the legacy
+  `max_follow = Inf` behaviour. Column names in the result also change — see
+  the mapping table below.
+
+  | Legacy column | General column |
+  |---------------|----------------|
+  | `y.sim`       | `y_sim`        |
+  | `event.sim`   | `event_sim`    |
+  | `treat`       | `treat_sim`    |
+  | `flag.harm`   | `flag_harm`    |
+
+### Deprecated parameters
+
+* `run_simulation_analysis(max_follow)` → use `analysis_time`. If supplied,
+  `max_follow` is silently forwarded to `analysis_time` with a warning.
+
+* `run_simulation_analysis(muC_adj)` → use `cens_adjust`. If supplied,
+  `muC_adj` is silently forwarded to `cens_adjust` with a warning.
+
+### Bug fixes
+
+The following bugs were discovered and fixed during the general pipeline
+migration. All affected code paths were exercised by GBSG factor variables
+(`v1`–`v7`) stored as `factor()` rather than `numeric()`.
+
+* `lasso_selection()` (`get_FSdata_helpers.R`): `as.matrix()` on a data frame
+  containing factor columns produced a character matrix that `cv.glmnet()`
+  rejected. Factor columns with all-numeric levels are now coerced via
+  `as.integer(as.character(.))` before matrix conversion.
+
+* `process_conf_force_expr()` (`get_FSdata_helpers.R`): `mean()` applied to a
+  factor column returned `NA`. Factor columns are now coerced to numeric before
+  `mean()`, `median()`, and `quantile()` calls.
+
+* `evaluate_comparison()` (`forestsearch_helpers.R`): the `<=` / `>=`
+  operator applied to a factor column triggered an `Ops.factor` warning and
+  returned `NA`. Factor columns are now coerced to numeric before comparison.
+
+* `forestsearch()` (`forestsearch_main.R`): `df[, conf.screen]` dropped to a
+  vector when `conf.screen` had length 1, causing `dummy()` to error on a
+  non-data-frame input. Fixed by adding `drop = FALSE`.
+
+* `default_grf_params_gen()` (`run_simulation_analysis.R`): `maxdepth` was
+  initialised to `4`, exceeding the maximum of `3` accepted by
+  `grf.subg.harm.survival()`. Corrected to `2` (matching the legacy default).
+
+* `default_grf_params_gen()` (`run_simulation_analysis.R`): `sg.criterion`
+  was set to `"hr"`, which is not a valid value. Corrected to `"mDiff"`
+  (matching the legacy default).
+
+### Internal changes
+
+* `create_gbsg_dgm()` and `simulate_from_gbsg_dgm()` are now thin public
+  wrappers that call `.create_gbsg_dgm_()` and `.simulate_from_gbsg_dgm_()`
+  internally. This prevents deprecation-warning spam in functions that call
+  these internally in loops or binary searches (`calibrate_k_inter()`,
+  `get_dgm_with_output()`, `validate_k_inter_effect()`).
+
+* `compute_dgm_cde()` now resolves the super-population data frame from
+  `dgm$df_super_rand` (GBSG DGMs) or `dgm$df_super` (general `aft_dgm_flex`
+  DGMs), making it compatible with both class hierarchies.
+
+* `globals.R`: added `"sim_id"` to `utils::globalVariables()` to suppress a
+  spurious `R CMD check` NOTE from `run_simulation_analysis.R`.
+
+---
+
+## forestsearch 0.1.0
+
+* Initial release.

R/forestsearch_helpers.R

@@ -169,6 +169,13 @@ evaluate_comparison <- function(expr, df) {
       }
 
       col <- df[[var_name]]
+      # Coerce factors with all-numeric levels to numeric so that
+      # numeric comparisons (<=, >=, <, >) are meaningful.
+      if (is.factor(col)) {
+        lvls <- levels(col)
+        if (!anyNA(suppressWarnings(as.numeric(lvls))))
+          col <- as.numeric(as.character(col))
+      }
       val <- suppressWarnings(as.numeric(value))
       if (is.na(val)) val <- value # keep as character for string comparisons
 

R/forestsearch_main.R

@@ -527,7 +527,7 @@ forestsearch <- function(df.analysis,
   # (e.g., q1 -> q1.0, q1.1), so the search explores BOTH directions
   # of each cut (subgroup and complement).
 
-  df.confounders <- df[, conf.screen]
+  df.confounders <- df[, conf.screen, drop = FALSE]
   df.confounders <- dummy(df.confounders)
 
 

R/get_FSdata_helpers.R

@@ -112,7 +112,19 @@ if (!requireNamespace("glmnet", quietly = TRUE)) stop("Package 'glmnet' is requi
 if (!requireNamespace("survival", quietly = TRUE)) stop("Package 'survival' is required.")
 
   # Prepare the design matrix and response
-  x <- as.matrix(df[, confounders.name, drop = FALSE])
+  # Coerce factor columns with all-numeric levels to integer so that
+  # as.matrix() produces a numeric matrix (not a character one).
+  # as.integer(as.character(f)) preserves the numeric value of each level
+  # (e.g. factor "1" -> 1L), unlike plain as.integer(f) which gives 1-based codes.
+  x_df <- df[, confounders.name, drop = FALSE]
+  for (.nm in names(x_df)) {
+    if (is.factor(x_df[[.nm]])) {
+      .lvls <- levels(x_df[[.nm]])
+      if (!anyNA(suppressWarnings(as.numeric(.lvls))))
+        x_df[[.nm]] <- as.integer(as.character(x_df[[.nm]]))
+    }
+  }
+  x <- as.matrix(x_df)
 
   y <- survival::Surv(df[[outcome.name]], df[[event.name]])
 
@@ -187,15 +199,18 @@ process_conf_force_expr <- function(expr, df) {
   arg <- matches[4]
   # Only proceed if var and arg match
   if (!(var %in% colnames(df)) || !(arg %in% colnames(df))) return(expr)
+  # Coerce factor columns to numeric before computing summary statistics
+  col_vals <- df[[arg]]
+  if (is.factor(col_vals)) col_vals <- as.numeric(as.character(col_vals))
   # Evaluate the function
   if (fun == "mean") {
-    val <- round(mean(df[[arg]], na.rm = TRUE), 1)
+    val <- round(mean(col_vals, na.rm = TRUE), 1)
   } else if (fun == "median") {
-    val <- round(median(df[[arg]], na.rm = TRUE), 1)
+    val <- round(median(col_vals, na.rm = TRUE), 1)
   } else if (fun == "qlow") {
-    val <- round(quantile(df[[arg]], 0.25, na.rm = TRUE), 1)
+    val <- round(quantile(col_vals, 0.25, na.rm = TRUE), 1)
   } else if (fun == "qhigh") {
-    val <- round(quantile(df[[arg]], 0.75, na.rm = TRUE), 1)
+    val <- round(quantile(col_vals, 0.75, na.rm = TRUE), 1)
   } else {
     # Unknown function, return as is
     return(expr)

R/globals.R

@@ -216,6 +216,7 @@ utils::globalVariables(c(
   "hr.Hc.true", "hr.Hc.hat",
   "hr.itt", "hr.adj.itt",
   "p.cens", "taumax",
+  "sim_id",
   "analysis",
   "trimmed",
   "aa",

R/oc_analyses_gbsg.R

@@ -209,9 +209,10 @@ get_dgm_hr <- function(dgm, which = "hr_H") {
 #' @export
 compute_dgm_cde <- function(dgm, harm_col = NULL) {
 
-  df <- dgm$df_super_rand
+  # Support both gbsg_dgm (df_super_rand) and aft_dgm_flex (df_super)
+  df <- dgm$df_super_rand %||% dgm$df_super
   if (is.null(df)) {
-    warning("DGM has no df_super_rand; cannot compute CDE.")
+    warning("DGM has no df_super_rand or df_super; cannot compute CDE.")
     return(dgm)
   }
   if (!all(c("theta_0", "theta_1") %in% names(df))) {
@@ -1277,8 +1278,13 @@ run_grf_analysis <- function(
 #'
 #' @importFrom data.table data.table rbindlist
 #' @importFrom stats rnorm
-#' @export
-run_simulation_analysis <- function(
+# Legacy GBSG-coupled implementation — retained as internal for backwards
+# compatibility. The exported run_simulation_analysis() now lives in
+# R/run_simulation_analysis.R and accepts any aft_dgm_flex DGM.
+# Parameters max_follow / muC_adj are forwarded as deprecated shims in the
+# general version.
+#' @keywords internal
+.run_simulation_analysis_legacy_ <- function(
     sim_id,
     dgm,
     n_sample,