Conversation
- Pass `loss` to the constructor as documented in https://www.adriangb.com/scikeras/stable/migration.html - Update package versions - Update syntax to use SciKeras compiled models
stsievert
left a comment
There was a problem hiding this comment.
Looks like the tests are failing because TF/Keras isn't installed on this branch. Could you comment out this line to make sure tests pass?
Line 53 in 0ea276d
The example on this doc page is tested in tests/model_selection/test_keras.py . Can you update that test too?
| $ pip install tensorflow>=2.3.0 | ||
| $ pip install scikeras>=0.1.8 | ||
| $ pip install tensorflow>=2.4.0 | ||
| $ pip install scikeras>=0.3.2 |
There was a problem hiding this comment.
Why is this required? Dask-ML will still work with the removed versions, right?
There was a problem hiding this comment.
Things should work generally, but some of the syntax in this tutorial may not. I think we should either update the versions, or remove them altogether (since not specifying a version usually gets you the latest version anyway).
There was a problem hiding this comment.
Dask-ML will not work with SciKeras v0.1.7. I think that version didn't have serialization (?).
We should make a note about the versioning. "The example below uses X. The usage with lower versions may be different than this example."
There was a problem hiding this comment.
Wasn't there also some issue about serialization of stateful optimizers like Adam?
There was a problem hiding this comment.
I'll add a note along the lines of #832 (comment)
Wasn't there also some issue about serialization of stateful optimizers like Adam?
Yeah, we fixed that in v0.3.0, which is another good reason to bump the "recommended" version numbers in these docs, although I don't think we want to mention that here right?
There was a problem hiding this comment.
we fixed [serialization] in v0.3.0
That's a really good reason to require SciKeras v0.3.0.
| from tensorflow.keras.layers import Dense | ||
| from tensorflow.keras.models import Sequential | ||
|
|
||
| def build_model(lr=0.01, momentum=0.9): |
|
The tests in e73daa9 were failing within TF (failure line). |
lossto the constructor as documented in https://www.adriangb.com/scikeras/stable/migration.html