Skip to content

Check compatibility of model spec and tailor - #304

Merged
hfrick merged 11 commits into
mainfrom
compatibility-model-post
Aug 20, 2025
Merged

hfrick merged 11 commits into
mainfrom
compatibility-model-post

Conversation

@hfrick

@hfrick hfrick commented Aug 19, 2025

Copy link
Copy Markdown
Member

closes #291

Parsnip model_specs can have a mode of "classification", "regression", "censored regression" or "unknown". When adding a model spec to a workflow, the mode cannot be unknown anymore.

Tailors can have a type of "regression", "binary", "multiclass" or "unknown". There is currently no way for the user to set the type directly, so workflows does not require the tailor to have a type other than unknown.

Tailors start with an unknown mode (via tailor::tailor()) and then the type gets inferred from the adjustments added.

When we add a tailor to a workflow, it will have

  • type = "unknown" only if it's a tailor without any adjustments, or all adjustments are adjust_predictions_custom().
  • type = "binary" for all classification problems, including multi-class. The type only gets updated during fit.tailor().
  • or otherwise: type = "regression"

Once it comes to fitting a tailor, we have the response, so we can update the tailor type

  • from "unknown" to one of the other ones
  • from "binary" to "multiclass" if appropriate

This PR checks what we can when specifying the workflow, in add_tailor(), add_model(), and workflows().

In those situations where fit.tailor() changes the tailor type, we can't really do anything earlier, or are fine already:

  • If the "unknown" tailor type gets changed, it was either a plain tailor (which is fine) or only custom adjustments (which we can't check).
  • If the type gets changed from "binary" to "multiclass", we already checked that the model mode is "classification".

@hfrick
hfrick requested a review from Copilot August 19, 2025 12:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds compatibility validation between parsnip model specifications and tailor postprocessors in workflows. The validation ensures that model modes (regression, classification, etc.) are compatible with tailor types (regression, binary, multiclass) before adding them to a workflow.

  • Adds compatibility checks in add_tailor(), add_model(), and workflow() functions
  • Implements validation logic to prevent incompatible model-tailor combinations
  • Adds comprehensive test coverage for the new validation functionality

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
R/workflow.R Updates error message format and adds validation call to add_postprocessor()
R/post-action-tailor.R Implements core compatibility validation logic between model specs and tailors
R/fit-action-model.R Adds validation when adding models to workflows with existing tailors
tests/testthat/test-workflow.R Tests compatibility validation and postprocessor validation in main workflow function
tests/testthat/test-post-action-tailor.R Tests compatibility validation when adding tailors to workflows
tests/testthat/test-fit-action-model.R Tests compatibility validation when adding models to workflows with tailors
tests/testthat/test-extract.R Updates test to use compatible model type for existing tailor
tests/testthat/_snaps/*.md Snapshot files for error message testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread R/post-action-tailor.R Outdated
Comment thread R/post-action-tailor.R Outdated
Comment thread R/fit-action-model.R Outdated
@hfrick
hfrick requested a review from topepo August 19, 2025 14:07
@hfrick
hfrick requested review from simonpcouch and removed request for topepo August 20, 2025 16:36

@simonpcouch simonpcouch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, so helpful to have that description as a refresher. Thank you for the thoroughness!

Looks great.

Comment thread R/post-action-tailor.R Outdated
Comment thread R/workflow.R
check_dots_empty()

if (is_tailor(postprocessor)) {
# check compatibility here for the right call in the error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

super helpful comment

recipes::step_pca(recipes::all_predictors(), threshold = hardhat::tune())
)
wflow <- add_model(wflow, parsnip::linear_reg())
wflow <- add_model(wflow, parsnip::logistic_reg())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tbh finding it very encouraging that this successfully surfaced an issue in tests

hfrick and others added 2 commits August 20, 2025 19:21
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
@hfrick
hfrick merged commit 5e0f14f into main Aug 20, 2025
@hfrick
hfrick deleted the compatibility-model-post branch August 27, 2025 10:53
@github-actions

Copy link
Copy Markdown

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions Bot locked and limited conversation to collaborators Sep 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent mismatch in model mode and tailor type

3 participants