Skip to content

Conversation

davidrsch
Copy link
Owner

Refactoring functional api to handle multi-input and multi-output correctly and updating guid to reflect so

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

air

[air] reported by reviewdog 🐶


folds <- rsample::vfold_cv(iris, v = 2)
params <- extract_parameter_set_dials(tune_wf) |>
params <- extract_parameter_set_dials(tune_wf) |>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
params <- extract_parameter_set_dials(tune_wf) |>
params <- extract_parameter_set_dials(tune_wf) |>

Comment on lines +137 to +138
input_block_1 <- function(input_shape) layer_input(shape = input_shape, name = "input_1")
input_block_2 <- function(input_shape) layer_input(shape = input_shape, name = "input_2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
input_block_1 <- function(input_shape) layer_input(shape = input_shape, name = "input_1")
input_block_2 <- function(input_shape) layer_input(shape = input_shape, name = "input_2")
input_block_1 <- function(input_shape) {
layer_input(shape = input_shape, name = "input_1")
}
input_block_2 <- function(input_shape) {
layer_input(shape = input_shape, name = "input_2")
}

flatten_b = inp_spec(flatten_block, "input_b"),
path_a = inp_spec(dense_path, "flatten_a"),
path_b = inp_spec(dense_path, "flatten_b"),
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
concatenated = inp_spec(
concat_block,
c(path_a = "in_1", path_b = "in_2")
),

expect_equal(names(preds), c(".pred_class"))
expect_equal(nrow(preds), 5)
expect_true(is.factor(preds$.pred_class))
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
})
})

Copy link

codecov bot commented Aug 16, 2025

Codecov Report

❌ Patch coverage is 70.10309% with 87 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
R/build_and_compile_model.R 72.58% 34 Missing ⚠️
R/register_fit_predict.R 50.81% 30 Missing ⚠️
R/utils.R 83.33% 10 Missing ⚠️
R/generic_fit_helpers.R 69.23% 8 Missing ⚠️
R/keras_tools.R 50.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

davidrsch and others added 17 commits August 16, 2025 15:59
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
davidrsch and others added 4 commits August 16, 2025 16:04
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@davidrsch davidrsch merged commit ec63e4b into main Aug 16, 2025
0 of 11 checks passed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

combined_preds <- purrr::map2_dfc(results, names(results), function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res)) # This might not be correct for classes, but a placeholder
}

block_name
]]) &&
length(y_processed$class_levels[[block_name]]) > 0,
num_classes = if (!is.null(y_processed$class_levels[[block_name]])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
num_classes = if (!is.null(y_processed$class_levels[[block_name]])) {
num_classes = if (
!is.null(y_processed$class_levels[[block_name]])
) {

block_hyperparams$num_classes <- current_y_info$num_classes
}
}
} else { # Single output case
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
} else { # Single output case
} else {
# Single output case

Comment on lines 159 to 163
combined_preds <- purrr::map2_dfc(results, names(results), function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
combined_preds <- purrr::map2_dfc(results, names(results), function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
combined_preds <- purrr::map2_dfc(
results,
names(results),
function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
}
colnames(res) <- lvls
tibble::as_tibble(res, .name_repair = "unique") %>%
dplyr::rename_with(~ paste0(".pred_", name, "_", .x))

Comment on lines +212 to +214
tibble::tibble(.pred_class = pred_class) %>%
dplyr::rename_with(~ paste0(".pred_class_", name))
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
tibble::tibble(.pred_class = pred_class) %>%
dplyr::rename_with(~ paste0(".pred_class_", name))
})
)

set_engine("keras")
fit_3 <- fit(spec_3, mpg ~ ., data = mtcars)
model_3_layers <- fit_3 |>
extract_keras_model() |>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
extract_keras_model() |>
extract_keras_model() |>

Comment on lines 120 to 121
output_block_1 <- function(tensor) layer_dense(tensor, units = 1, name = "output_1")
output_block_2 <- function(tensor) layer_dense(tensor, units = 1, name = "output_2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
output_block_1 <- function(tensor) layer_dense(tensor, units = 1, name = "output_1")
output_block_2 <- function(tensor) layer_dense(tensor, units = 1, name = "output_2")
output_block_1 <- function(tensor) {
layer_dense(tensor, units = 1, name = "output_1")
}
output_block_2 <- function(tensor) {
layer_dense(tensor, units = 1, name = "output_2")
}

input_b = input_block_2,
path_a = inp_spec(dense_path, "input_a"),
path_b = inp_spec(dense_path, "input_b"),
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
concatenated = inp_spec(
concat_block,
c(path_a = "in_1", path_b = "in_2")
),

expect_equal(nrow(preds), 5)
expect_true(is.numeric(preds$.pred_output_1))
expect_true(is.numeric(preds$.pred_output_2))
}) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
})
})

block_name
]]) &&
length(y_processed$class_levels[[block_name]]) > 0,
num_classes = if (!is.null(y_processed$class_levels[[block_name]])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
num_classes = if (!is.null(y_processed$class_levels[[block_name]])) {
num_classes = if (
!is.null(y_processed$class_levels[[block_name]])
) {

block_hyperparams$num_classes <- current_y_info$num_classes
}
}
} else { # Single output case
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
} else { # Single output case
} else {
# Single output case

]
merged_args
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
}
}

Comment on lines 159 to 163
combined_preds <- purrr::map2_dfc(results, names(results), function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
combined_preds <- purrr::map2_dfc(results, names(results), function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
combined_preds <- purrr::map2_dfc(
results,
names(results),
function(res, name) {
lvls <- object$fit$lvl[[name]] # Assuming object$fit$lvl is a named list of levels
if (is.null(lvls)) {
# Fallback if levels are not specifically named for this output
lvls <- paste0("class", 1:ncol(res))
}
colnames(res) <- lvls
tibble::as_tibble(res, .name_repair = "unique") %>%
dplyr::rename_with(~ paste0(".pred_", name, "_", .x))

Comment on lines +165 to +168
colnames(res) <- lvls
tibble::as_tibble(res, .name_repair = "unique") %>%
dplyr::rename_with(~ paste0(".pred_", name, "_", .x))
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
colnames(res) <- lvls
tibble::as_tibble(res, .name_repair = "unique") %>%
dplyr::rename_with(~ paste0(".pred_", name, "_", .x))
})
)

set_engine("keras")
fit_3 <- fit(spec_3, mpg ~ ., data = mtcars)
model_3_layers <- fit_3 |>
extract_keras_model() |>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
extract_keras_model() |>
extract_keras_model() |>

Comment on lines 114 to 115
input_block_1 <- function(input_shape) layer_input(shape = input_shape, name = "input_1")
input_block_2 <- function(input_shape) layer_input(shape = input_shape, name = "input_2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
input_block_1 <- function(input_shape) layer_input(shape = input_shape, name = "input_1")
input_block_2 <- function(input_shape) layer_input(shape = input_shape, name = "input_2")
input_block_1 <- function(input_shape) {
layer_input(shape = input_shape, name = "input_1")
}
input_block_2 <- function(input_shape) {
layer_input(shape = input_shape, name = "input_2")
}

Comment on lines 120 to 121
output_block_1 <- function(tensor) layer_dense(tensor, units = 1, name = "output_1")
output_block_2 <- function(tensor) layer_dense(tensor, units = 1, name = "output_2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
output_block_1 <- function(tensor) layer_dense(tensor, units = 1, name = "output_1")
output_block_2 <- function(tensor) layer_dense(tensor, units = 1, name = "output_2")
output_block_1 <- function(tensor) {
layer_dense(tensor, units = 1, name = "output_1")
}
output_block_2 <- function(tensor) {
layer_dense(tensor, units = 1, name = "output_2")
}

input_b = input_block_2,
path_a = inp_spec(dense_path, "input_a"),
path_b = inp_spec(dense_path, "input_b"),
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
concatenated = inp_spec(concat_block, c(path_a = "in_1", path_b = "in_2")),
concatenated = inp_spec(
concat_block,
c(path_a = "in_1", path_b = "in_2")
),

expect_equal(nrow(preds), 5)
expect_true(is.numeric(preds$.pred_output_1))
expect_true(is.numeric(preds$.pred_output_2))
}) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[air] reported by reviewdog 🐶

Suggested change
})
})

@davidrsch davidrsch deleted the functional-api-guide branch August 16, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant