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

modify parameters in-place in sciml_train! #140

Closed
ric-cioffi opened this issue Feb 5, 2020 · 4 comments
Closed

modify parameters in-place in sciml_train! #140

ric-cioffi opened this issue Feb 5, 2020 · 4 comments

Comments

@ric-cioffi
Copy link
Contributor

This is more of a suggestion rather than an issue.
I'd find it much more intuitive to have sciml_train! to modify the parameters in-place, which would be consistent with Flux.train!'s behavior.

@ChrisRackauckas
Copy link
Member

I am going the complete other direction in #139 and just renaming it sciml_train instead of sciml_train!. One of the purposes of this was to get rid of the global behavior because it's brittle, not thread-safe, and can lead to some weird performance issues.

@ric-cioffi
Copy link
Contributor Author

I'm totally fine with this as well, but it should probably be highlighted in the docs that DiffEqFlux.sciml_train behaves differently from Flux.train! even when called with a Flux optimizer.

@ChrisRackauckas
Copy link
Member

Yes, it does behave differently from Flux.train! in more than one way, another being after #139 the boolean in the callback. Another is the maxiters stuff. So I think we just break the link and don't look back. People can still use Flux.train! if they want, but we can just give an interface that's better for the kinds of applications we see.

@ChrisRackauckas
Copy link
Member

Went the route of #139

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

No branches or pull requests

2 participants