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

Lux compatible layers #750

Merged
merged 27 commits into from
Jan 11, 2023
Merged

Lux compatible layers #750

merged 27 commits into from
Jan 11, 2023

Conversation

ba2tro
Copy link
Contributor

@ba2tro ba2tro commented Jul 11, 2022

No description provided.

src/neural_de.jl Outdated Show resolved Hide resolved
src/neural_de.jl Outdated Show resolved Hide resolved
src/neural_de.jl Outdated Show resolved Hide resolved
src/neural_de.jl Show resolved Hide resolved
src/neural_de.jl Outdated Show resolved Hide resolved
@ChrisRackauckas
Copy link
Member

What's the status of this?

@ba2tro
Copy link
Contributor Author

ba2tro commented Oct 23, 2022

NeuralDSDE and NeuralSDE with Lux work now using EM or LambaEM with BacksolveAdjoint. But the AD vjp s are throwing some errors. It works only with numerical vjp right now. I think we'll need some changes to SciMLSensitivity to make vjp compatible?

@ChrisRackauckas
Copy link
Member

How is it erroring? Use EulerHeun so it's in Stratanovich form: that's simpler for training.

ba2tro added a commit to ba2tro/StochasticDiffEq.jl that referenced this pull request Dec 11, 2022
While using `EulerHeun` with `BacksolveAdjoint` there's this mul! issues due to W.dW being a matrix. Doing `vec()` fixes it. Downstream: SciML/DiffEqFlux.jl#750 (comment)
@ba2tro ba2tro marked this pull request as ready for review January 4, 2023 11:28
src/neural_de.jl Outdated Show resolved Hide resolved
@ChrisRackauckas
Copy link
Member

@avik-pal
Copy link
Member

avik-pal commented Jan 6, 2023

I had never stored parameters in the layers so never tested @functor. The fallback definition there is to allow recursively traverse container layers.

For Flux compat, I think we should directly define the Optimisers.trainable though that will not allow Flux.gpu to work as expected.

@ChrisRackauckas
Copy link
Member

But it seems it shouldn't dispatch on the second argument if it's going to work with @functor?

@avik-pal
Copy link
Member

avik-pal commented Jan 6, 2023

LuxDL/Lux.jl#229 should fix it. I will tag a release once the CI passes

@ChrisRackauckas
Copy link
Member

@Abhishek-1Bhatt https://github.com/SciML/DiffEqFlux.jl/actions/runs/3855851152/jobs/6581195485 this failure looks real, but looks like it just needs a functor overload.

@ba2tro
Copy link
Contributor Author

ba2tro commented Jan 9, 2023

The error message, seems to be referring to model as in NeuralDELayer <: Lux.AbstractExplicitContainerLayer{(:model,)} as HamiltonianNeuralDE is also subtyped to NeuralDELayer here , maybe doing something like this could help?

@ChrisRackauckas
Copy link
Member

yes the naming seems wrong, so it either needs to rename to just model or have another abstract type. I would recommend just naming it model like the others, I think that might make the structure simpler.

test/runtests.jl Outdated Show resolved Hide resolved
@ChrisRackauckas ChrisRackauckas merged commit c30246e into SciML:master Jan 11, 2023
@ba2tro ba2tro deleted the lux branch January 20, 2023 10:48
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

Successfully merging this pull request may close these issues.

3 participants