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

refactor: trigger build with latest Lux #882

Merged
merged 57 commits into from
Oct 17, 2024
Merged

refactor: trigger build with latest Lux #882

merged 57 commits into from
Oct 17, 2024

Conversation

avik-pal
Copy link
Member

No description provided.

@avik-pal avik-pal force-pushed the ap/trigger branch 7 times, most recently from 0c724f7 to b25ed28 Compare August 21, 2024 08:04
@avik-pal
Copy link
Member Author

avik-pal commented Aug 21, 2024

@ChrisRackauckas I tracked down and fixed the upstream issues. The only one remaining is https://github.com/SciML/NeuralPDE.jl/actions/runs/10485955321/job/29043159135?pr=882#step:6:1216 but that is kind of hard to handle -- it is matrix multiply with non-concrete element types.

Might be best to get the adapt usage fixed on NeuralPDE end that will fix all the issues

@sathvikbhagavan
Copy link
Member

Might be best to get the adapt usage fixed on NeuralPDE end that will fix all the issues

I can look into it. Can you open an issue about it?

Also, in CI, why is precompilation failing - https://github.com/SciML/NeuralPDE.jl/actions/runs/10485955321/job/29043160310?pr=882#step:6:982

@avik-pal
Copy link
Member Author

avik-pal commented Aug 21, 2024

I can look into it. Can you open an issue about it?

#883

Also, in CI, why is precompilation failing - SciML/NeuralPDE.jl/actions/runs/10485955321/job/29043160310?pr=882#step:6:982

Depwarn errors are on, and currently Enzyme doesn't support the latest GPUCompiler which doesn't have those depwarns (same as SciML/DeepEquilibriumNetworks.jl#159)

@ChrisRackauckas
Copy link
Member

Might be best to get the adapt usage fixed on NeuralPDE end that will fix all the issues

That's the right thing to do.

@ChrisRackauckas
Copy link
Member

@wsmoses what should we do about the GPUCompiler Enzyme depwarn issue? How close is that to getting the release?

@wsmoses
Copy link

wsmoses commented Aug 26, 2024

Enzyme released a version with the latest gpucompiler last night

@ChrisRackauckas
Copy link
Member

@avik-pal you're unblocked here?

@avik-pal
Copy link
Member Author

avik-pal commented Sep 1, 2024

No I need #883 to be fixed, else it tries to propagate Matrix{Any}

@ChrisRackauckas
Copy link
Member

Okay, but at least unblocked from non-NeuralPDE things 😅 . @KirillZubov do you think you can have a look at that?

@KirillZubov
Copy link
Member

@ChrisRackauckas ok, I will try figure out with it , it's a little related to my current task

@@ -217,20 +217,20 @@ function generate_training_sets(domains, dx, eqs, bcs, eltypeθ, dict_indvars::D

bcs_train_sets = map(bound_args) do bt
span = map(b -> get(dict_var_span, b, b), bt)
_set = adapt(eltypeθ,
_set = adapt(Array{eltypeθ},
Copy link
Member

Choose a reason for hiding this comment

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

Do not force an Array type, since that won't work in most scenarios. Just add a post-adapt convert.(eltypeθ, x)

@KirillZubov
Copy link
Member

I updated this PR also to Lux v1.0 #887

@KirillZubov KirillZubov removed the request for review from sathvikbhagavan October 10, 2024 06:18
@KirillZubov
Copy link
Member

@sathvikbhagavan @ChrisRackauckas Unfortunately, I couldn't cope with this task and I don't have time to do it in the near future. In addition to updating the packages(Lux to v1 ) that I've already done, It also need to review all the tests and update the solvers which is need some time on it. I fixed the problem with Adapt and Convert types(#883) in a couple of places. This template can be used for other solvers. There are also other errors, the cause of which I haven't had time to establish.

@avik-pal avik-pal marked this pull request as ready for review October 16, 2024 02:13
@avik-pal avik-pal linked an issue Oct 16, 2024 that may be closed by this pull request
@avik-pal
Copy link
Member Author

need to close #885 #893 #886

@avik-pal avik-pal changed the title chore: trigger build with latest Lux refactor: trigger build with latest Lux Oct 16, 2024
@avik-pal
Copy link
Member Author

Lets merge once all the tests pass. I think docs build might fail due to linkchecks but it also takes like 17hrs. I will try to go through and see if we can reduce the time there.

@avik-pal
Copy link
Member Author

@avik-pal
Copy link
Member Author

I am going to merge and fix the docs in a followup PR

@avik-pal avik-pal merged commit 4506f6a into master Oct 17, 2024
29 of 33 checks passed
@avik-pal avik-pal deleted the ap/trigger branch October 17, 2024 00:30
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.

Iteration should be a Ref{Int}, not a size 1 Vector{Int}
5 participants