-
-
Notifications
You must be signed in to change notification settings - Fork 76
refactor: make lib/* into extensions #517
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
Conversation
45edbfe to
ca8014c
Compare
ca8014c to
4e8be11
Compare
|
@ChrisRackauckas, can you take a look? I have converted all the lib/ packages into extensions. After this, I will update the docs in a separate PR. |
|
Looks fine but the docs need to be updated to not be importing the old libraries. |
d865807 to
5548631
Compare
020585a to
c12c45f
Compare
fb17003 to
7f205da
Compare
|
@ChrisRackauckas, I have fixed the docs and downgrade CI as well. |
Project.toml
Outdated
|
|
||
| [extensions] | ||
| SurrogatesAbstractGPsExt = "AbstractGPs" | ||
| SurrogatesFluxExt = ["Flux", "Optimisers"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimisers being necessary will need to be clearly documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i will document it.
ext/SurrogatesFluxExt.jl
Outdated
|
|
||
| using Surrogates: NeuralSurrogate | ||
| using SurrogatesBase | ||
| using Optimisers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimizers is in Flux though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I thought that was deprecated in favour of using Optimisers.jl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still baked in there, so that makes the extension easier.
Co-authored-by: Sathvik Bhagavan <[email protected]>
Fixes: #513