add batch_size = 'all' option?

[john-veillette]

Hey devs, thanks for this package.

I see that **kwargs is being deprecated from .fit, which makes sense in the interest of making the API more consistent for grid search, etc. However, it still seems desirable to be able to specify batch_size based on the input dimensions in case users want to use vanilla rather than stochastic gradient descent. Maybe adding a batch_size = 'all' option in init would remove any need to specify batch_size in fit?

[adriangb]

Hi 👋, glad you're finding it useful!

Would all=X.shape[0]?

[john-veillette]

Yep! To replace the option to specify model.fit(X, y, batch_size = X.shape[0]) when fit(X, y, **kwargs) is removed, one could specify batch_size = 'all' on initialization. Should only take a few lines of code to implement.

[adriangb]

Agreed. Will leave this issue open to track.

[john-veillette]

Thanks!

[stsievert]

Thats a decent argument for keeping fit **kwargs. I think compatibility with Keras is important.

[adriangb]

Hey @john-veillette, I haven't implemented this yet because like @stsievert points out above, we may ended up keeping **kwargs.

That said, I do think there are some use cases where **kwargs doesn't cut it. The first that comes to mind is cross-val-score, where presumably the length of the dataset is changing but you have no control over the sizes / cannot pass `**kwargs.

For your use case, do you feel that you actually need batch_size="all" or could **kwargs work for you?

Edit: I did a sample implementation, for reference, in #194.

[adriangb]

In addition to the implementation in #194, this could also be implemented via #167. Reference implementation here: https://www.adriangb.com/scikeras/refs/pull/167/merge/notebooks/DataTransformers.html#7.-Dynamically-setting-batch_size

[john-veillette]

For my use case, **kwargs works just as well! That's how I currently have it implemented, anyhow.

[adriangb]

Implemented in #194