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.

Sign up for GitHub

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

Jump to bottom

Fix Tests and Cleanup #873

Merged
merged 30 commits into from
Nov 22, 2023
Merged

Fix Tests and Cleanup #873

merged 30 commits into from
Nov 22, 2023

Conversation

avik-pal
Copy link
Member

@avik-pal avik-pal commented Nov 9, 2023 •
edited by ChrisRackauckas
Loading

Fixes #781 Fixes #795 Fixes #639 Fixes #638, Fixes #707 Fixes #733

@prbzrg
Copy link
Member

prbzrg commented Nov 9, 2023

I think if we are removing Flux, we should also remove Functors and trainable dispatches.

@prbzrg
Copy link
Member

prbzrg commented Nov 9, 2023

I think JuliaDiff/ChainRules.jl#713 is related.

@avik-pal avik-pal mentioned this pull request Nov 10, 2023
Merged
@prbzrg
Copy link
Member

prbzrg commented Nov 15, 2023

Fixes #781

@prbzrg
Copy link
Member

prbzrg commented Nov 15, 2023

Fixing the AD problem closes #814

@prbzrg
Copy link
Member

prbzrg commented Nov 15, 2023

Supporting only Lux makes #795 irrelevant and closes it.

This was linked to issues Nov 15, 2023
Use Lux.Experimental.StatefulLuxLayer internally to avoid Boxing #855
Closed
ERROR: UndefVarError: Chain not defined #795
Closed
Mutating arrays is not supported #814
Closed
@prbzrg
Copy link
Member

prbzrg commented Nov 15, 2023

Now that we are breaking some APIs, should we consider #790 ?
IMO, having another function to generate ODEProblem and use it in the current function fixed issues like this that user wants control over internal information.

@avik-pal avik-pal force-pushed the ap/thepurge branch 3 times, most recently from ba86fb6 to a072fd8 Compare November 16, 2023 03:58
@avik-pal avik-pal linked an issue Nov 16, 2023 that may be closed by this pull request
GPU broadcast resulted in non-concrete element type Any when running several NODEs in a single training #852
Closed
@avik-pal avik-pal removed a link to an issue Nov 16, 2023
GPU broadcast resulted in non-concrete element type Any when running several NODEs in a single training #852
Closed
@ChrisRackauckas
Copy link
Member

Maybe fix the DAEs

What's this part?

@ChrisRackauckas ChrisRackauckas merged commit 1c5c896 into SciML:master Nov 22, 2023
11 of 13 checks passed
ChrisRackauckas
ChrisRackauckas reviewed Nov 22, 2023
View reviewed changes
src/ffjord.jl
Comment on lines +132 to +134
mz, pb_f = Zygote.pullback(model, z, p)
e = CRC.@ignore_derivatives randn!(similar(mz))
eJ = first(pb_f(e))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should really change this to forward mode.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added ADTypes here so we can switch the internal AD using ad = AutoForwardDiff

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using JacVec or VecJac from SparseDiffTools, we can pass ad to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah what I mean is that it should be using JacVec (and almost never VecJac). It's e'Je, and so it should essentially always calculate e' * (Je) and not (e'J) * e which is what it does now (along with the PyTorch code)

@avik-pal avik-pal deleted the ap/thepurge branch November 22, 2023 16:20
@avik-pal
Copy link
Member Author

What's this part?

The DAEs didnot have any tests running even before, and I couldn't get AD to work currently

@ChrisRackauckas
Copy link
Member

Which form? The mass matrix?

@avik-pal
Copy link
Member Author

MM form (NeuralODEMM) works. the NeuralDAE form

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisRackauckas ChrisRackauckas ChrisRackauckas left review comments

@prbzrg prbzrg prbzrg left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@avik-pal @prbzrg @ChrisRackauckas