Fixing detected issue witth num_layer = 0 and making tests more robust

Author: davidrsch

R/generic_functional_fit.R

@@ -128,7 +128,15 @@ generic_functional_fit <- function(
 
     # --- Get Repetition Count ---
     num_repeats_arg <- paste0("num_", block_name)
-    num_repeats <- all_args[[num_repeats_arg]] %||% 1
+    num_repeats_val <- all_args[[num_repeats_arg]]
+
+    # If num_repeats_val is NULL or zapped, default to 1.
+    # Otherwise, use the value provided by the user.
+    if (is.null(num_repeats_val) || inherits(num_repeats_val, "rlang_zap")) {
+      num_repeats <- 1
+    } else {
+      num_repeats <- as.integer(num_repeats_val)
+    }
 
     # --- Get Hyperparameters for this block ---
     # Hyperparameters are formals that are NOT other block names (graph connections)

R/generic_sequential_fit.R

@@ -118,7 +118,14 @@ generic_sequential_fit <- function(
 
     num_repeats_arg <- paste0("num_", block_name)
     num_repeats_val <- all_args[[num_repeats_arg]]
-    num_repeats <- num_repeats_val %||% 1
+
+    # If num_repeats_val is NULL or zapped, default to 1.
+    # Otherwise, use the value provided by the user.
+    if (is.null(num_repeats_val) || inherits(num_repeats_val, "rlang_zap")) {
+      num_repeats <- 1
+    } else {
+      num_repeats <- as.integer(num_repeats_val)
+    }
 
     # Get the arguments for this specific block from `...`
     block_arg_names <- names(block_fmls)[-1] # Exclude 'model'

tests/testthat/test-e2e-features.R

@@ -102,14 +102,19 @@ test_that("E2E: Customizing fit arguments works", {
   expect_lt(length(fit_obj$fit$history$metrics$loss), 5)
 })
 
-test_that("E2E: Setting num_blocks = 0 works", {
+test_that("E2E: Setting num_blocks = 0 works for sequential models", {
   skip_if_no_keras()
 
   input_block_zero <- function(model, input_shape) {
     keras3::keras_model_sequential(input_shape = input_shape)
   }
   dense_block_zero <- function(model, units = 16) {
-    model |> keras3::layer_dense(units = units, activation = "relu")
+    model |>
+      keras3::layer_dense(
+        units = units,
+        activation = "relu",
+        name = "i_should_not_exist"
+      )
   }
   output_block_zero <- function(model) {
     model |> keras3::layer_dense(units = 1)
@@ -128,10 +133,18 @@ test_that("E2E: Setting num_blocks = 0 works", {
     mode = "regression"
   )
 
-  spec <- e2e_mlp_zero(num_dense = 0, fit_epochs = 2) |>
+  spec <- e2e_mlp_zero(num_dense = 0, fit_epochs = 1) |>
     parsnip::set_engine("keras")
-  # This should fit a model with only an input and output layer
-  expect_no_error(parsnip::fit(spec, mpg ~ ., data = mtcars))
+
+  fit_obj <- parsnip::fit(spec, mpg ~ ., data = mtcars)
+
+  # Check that the dense layer is NOT in the model
+  keras_model <- fit_obj |> extract_keras_summary()
+  expect_equal(length(keras_model$layers), 1) # Output layers only
+
+  # Check layer names explicitly
+  layer_names <- sapply(keras_model$layers, function(l) l$name)
+  expect_false("i_should_not_exist" %in% layer_names)
 })
 
 test_that("E2E: Error handling for reserved names works", {

tests/testthat/test-e2e-functional.R

@@ -182,3 +182,57 @@ test_that("E2E: Functional spec tuning (including repetition) works", {
   expect_s3_class(metrics, "tbl_df")
   expect_true(all(c("num_dense_path", "dense_path_units") %in% names(metrics)))
 })
+
+test_that("E2E: Block repetition works for functional models", {
+  skip_if_no_keras()
+
+  input_block <- function(input_shape) keras3::layer_input(shape = input_shape)
+  dense_block <- function(tensor, units = 8) {
+    tensor |> keras3::layer_dense(units = units, activation = "relu")
+  }
+  output_block <- function(tensor) keras3::layer_dense(tensor, units = 1)
+
+  model_name <- "e2e_func_repeat"
+  on.exit(suppressMessages(remove_keras_spec(model_name)), add = TRUE)
+
+  create_keras_functional_spec(
+    model_name = model_name,
+    layer_blocks = list(
+      main_input = input_block,
+      dense_path = inp_spec(dense_block, "main_input"),
+      output = inp_spec(output_block, "dense_path")
+    ),
+    mode = "regression"
+  )
+
+  # --- Test with 1 repetition ---
+  spec_1 <- e2e_func_repeat(num_dense_path = 1, fit_epochs = 1) |>
+    set_engine("keras")
+  fit_1 <- fit(spec_1, mpg ~ ., data = mtcars)
+  model_1_layers <- fit_1 |>
+    extract_keras_summary() |>
+    pluck("layers")
+
+  # Expect 3 layers: Input, Dense, Output
+  expect_equal(length(model_1_layers), 3)
+
+  # --- Test with 2 repetitions ---
+  spec_2 <- e2e_func_repeat(num_dense_path = 2, fit_epochs = 1) |>
+    set_engine("keras")
+  fit_2 <- fit(spec_2, mpg ~ ., data = mtcars)
+  model_2_layers <- fit_2 |>
+    extract_keras_summary() |>
+    pluck("layers")
+  # Expect 4 layers: Input, Dense, Dense, Output
+  expect_equal(length(model_2_layers), 4)
+
+  # --- Test with 0 repetitions ---
+  spec_3 <- e2e_func_repeat(num_dense_path = 0, fit_epochs = 1) |>
+    set_engine("keras")
+  fit_3 <- fit(spec_3, mpg ~ ., data = mtcars)
+  model_3_layers <- fit_3 |>
+    extract_keras_summary() |>
+    pluck("layers")
+  # Expect 2 layers: Input, Output
+  expect_equal(length(model_3_layers), 2)
+})