ModelOriented / ingredients

Effects and Importances of Model Ingredients
https://modeloriented.github.io/ingredients/
GNU General Public License v3.0
37 stars 18 forks source link

Pass dots to predict_function #143

Closed jmaspons closed 1 year ago

jmaspons commented 2 years ago

Use case: speed up predictions with keras by passing a larger batch_size parameter (32 by default)

hbaniecki commented 2 years ago

Thanks, how about passing parameters to the loss_function as well? Would it interfere?

jmaspons commented 2 years ago

Thanks, how about passing parameters to the loss_function as well? Would it interfere?

I think it's not a problem while there are no matching parameter names. I'll add the dots to loss_function in this PR if you agree

jmaspons commented 2 years ago

From ?DALEX::loss_default I don't see a use case for ... in loss_function but here it is. We can revert the last commit before merging if needed

hbaniecki commented 2 years ago

Sure, I meant if someone passes custom loss_function.

It would be useful to have a unit test for this custom predict/loss function with ....

I plan to merge this PR over the weekend.

jmaspons commented 2 years ago

I've been testing the changes and it doesn't work. It needs something like https://community.rstudio.com/t/dots-vs-arg-lists-for-function-forwarding/4995/2 to pass dots to 2 functions

jmaspons commented 2 years ago

Reveret last commit to also pass ... to loss_function. Can be implemented in another PR following https://community.rstudio.com/t/dots-vs-arg-lists-for-function-forwarding/4995/2

pbiecek commented 1 year ago

@hbaniecki shall we merge this one?