Skip to content

Allow layer_predict() to pass along ... #46

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

Closed
dajmcdon opened this issue Jun 11, 2022 · 4 comments · Fixed by #358
Closed

Allow layer_predict() to pass along ... #46

dajmcdon opened this issue Jun 11, 2022 · 4 comments · Fixed by #358
Assignees

Comments

@dajmcdon
Copy link
Contributor

On the frosting branch, there's a layer_predict() function. The dots need to be converted to a list, then passed back to the predict() call in the slather method. I tried a few different implementations but couldn't get it to work.

  • In the user facing layer_predict() function, the dots must be converted as far as I can tell (not simply passed along). This is because each layer is saved into a list. The current implementation is fine here.
  • But the dots_list would then need to be evaluated/defused on line 37. I tried combinations of rlang things. I also tried using do.call() with a list of args. I couldn't make either work (and the do.call version would seem to have undesirable copy behaviour).

@brookslogan I'm assigning you since you seem to have the most facility with quosure behaviour. I suspect there's a simple fix, but it's beyond me.

@brookslogan
Copy link
Contributor

brookslogan commented Jun 27, 2022

Sorry, just getting to this.

  • Can the dots be immediately evaluated in layer_predict? If not, can they be evaluated multiple times?
  • Do you want the dots to be evaluated like normal arguments, or do you need to manipulate the environments or make .data and .env pronouns available to the user?

A few guesses at what would work:

  • Immediately evaluate like normal arguments: use list, list2, dots_list, lst, or alist in layer_predict. In slather use rlang::inject(stats::predict(<stuff>, !!!object[["dots_list"]])).
  • Evaluate on slather, potentially multiple times, like normal arguments: use enquos0 or enquos. In slather use rlang::inject(stats::predict(<stuff>, !!!lapply(object[["dots"]], rlang::eval_tidy))).
  • Evaluate on slather, potentially multiple times, with environment manipulation or pronouns: same as previous bullet point but with appropriate arguments to eval_tidy.
  • Evaluate on slather, don't evaluate multiple times, with/without environment manipulation: we'd need a (custom?) approach where the dots would be an environment with some promise (delayedAssign) bindings. [Or maybe just use the enquos[0] approach and require all the dots to be evaluated on the first slather; the first slather would replace the enquosed dots with evaluated dots, and subsequent slathers would use the evaluated dots.]

@dajmcdon
Copy link
Contributor Author

Your comment illustrates exactly how little I know about this. The dots should be "additional named arguments passed on to the predict method". So I think the answers are:

  • Yes? Not evaluated multiple times.
  • Normal arguments?

I suspect this means that Bullet 1 should work (or something like it). However I tried some simpler versions, and ... no dice.

I want to be able to capture the dots into a list (as with dots <- list(...) ) and pass that in the dots_list argument to layer(). Then, the stats::predict(object,...) generic should have the dots_list passed as named arguments in the ....

I think that's bullet 1 right?

@brookslogan
Copy link
Contributor

brookslogan commented Jun 28, 2022

Okay, I'll take a look at implementing Bullet 1 and see what might be blocking it. (Terminology note: in this case, we probably won't say that the dots list is "evaluated" or "defused" in slather (I'm assuming this was line 37); as I understand it, "evaluation" in this context would take some sort of quoted thing and turn it into a value (eval{,_tidy,_bare}), and "defusing" would be using a quoting-like function (quote, [en]quo[s][0], expression, and so on). [Instead I think some languages call what slather should be doing "splatting" or "unpacking".])

I still haven't read through much of epipredict; can you please point me to the file you were showing with a workflow+frosting example? I'm having trouble grepping for it.

Are users directly setting some hyperparameters to be passed to a predict implementation here? E.g.,

<frosting object> %>%
  layer_predict(<type>, <opts>, lambda=42, alpha=0.2)

Do we want this to work on lm/glmnet-like fit objects, or some sort of things from workflows, recipes, parsnip, ...? Is this supposed to replace opts, or what's the interaction supposed to be here? Is this moving away from our prior decision to favor (validated) config lists rather than dots? [Case lm/glmnet, replace, yes --> probably want list or dots_list with some checks enabled and/or extra checks. Other situations might require more thought.]

@dajmcdon dajmcdon mentioned this issue Jul 6, 2022
@brookslogan
Copy link
Contributor

brookslogan commented Jul 9, 2024

Sorry, forgot that I was assigned here. I'm confused by recent discussion on Slack; is the goal to be able to record ... fed into layer_predict(), or to forward ... from predict() into slather.layer_{whatever}() including slather.layer_predict()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants