Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workflows #13

Closed
kbodwin opened this issue Mar 8, 2022 · 2 comments · Fixed by #143
Closed

Workflows #13

kbodwin opened this issue Mar 8, 2022 · 2 comments · Fixed by #143
Labels
Waiting ⌛ These tasks needs to be handled in a different package

Comments

@kbodwin
Copy link
Collaborator

kbodwin commented Mar 8, 2022

I think the next short-term step is to let these models be part of a workflow with a recipe. Right now there's an error from add_model(): "Error: spec must be a model_spec."

I'm guessing that cluster_spec can't (and shouldn't?) be a subclass of model_spec, so that would mean that either add_model() needs a S3 method for cluster specs, or we need a add_celery_model()

@EmilHvitfeldt EmilHvitfeldt added the Short term High priority changes label Mar 8, 2022
@DavisVaughan
Copy link
Member

Some thoughts:

  • workflows has been defined in "stages" to sort of account for this possibility. There is a "fit stage" where you can swap out the thing used to fit. So I think we are going to have a fit-action-celery.R file dedicated to celery, like how there is a fit-action-model.R file dedicated to parsnip model specs. This allows celery to differ from parsnip when it matters, and we can easily account for those differences in workflows because there is a dedicated code path just for celery.
  • control_workflow() currently has a control_parsnip argument. I think we could rename this to control to make it more generic, and allow a control_celery object through here too (it would become agnostic to the type of the actual control object and rely on the underlying fit() method to check the control object type)
  • add_variables() requires the outcomes argument, but you can do outcomes = NULL to say "no outcome". I think that might be the best we can do there

@EmilHvitfeldt EmilHvitfeldt added Waiting ⌛ These tasks needs to be handled in a different package feature a feature request or enhancement and removed Short term High priority changes feature a feature request or enhancement labels Jul 10, 2022
@github-actions
Copy link

This issue 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 Aug 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Waiting ⌛ These tasks needs to be handled in a different package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants