-
-
Notifications
You must be signed in to change notification settings - Fork 238
fix GPU compat of BrownFullBasicInit #2911
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
base: master
Are you sure you want to change the base?
Conversation
| algebraic_vars = mapreduce(iszero, &, M, dims = 1)[:] | ||
| algebraic_eqs = mapreduce(iszero, &, M, dims = 2)[:] |
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.
| algebraic_vars = mapreduce(iszero, &, M, dims = 1)[:] | |
| algebraic_eqs = mapreduce(iszero, &, M, dims = 2)[:] | |
| algebraic_vars = mapreduce(iszero, &, M, dims = 1) | |
| algebraic_eqs = mapreduce(iszero, &, M, dims = 2) |
Is that part needed?
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 think so, otherwise those are not vectors but 1xN and Nx1 matrices, the rest of the code assumes vectors. But i'll check again
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.
the vector is indeed needed. I changed it to
algebraic_vars = vec(all(iszero, M, dims = 1))
algebraic_eqs = vec(all(iszero, M, dims = 2))which is more readable and faster.
|
Hm the new testcase fails in CI weird, can't reproduce this locally, there the fix works |
This PR adds a simple (mutating) DAE as a testcase for GPU compat. Mainly fixes BrownInit for mutating DAEs with mass matrix and cu arrays.
Not tested:
jac_prototypedoes not work, fails in rosenbrock init because of Default algs for CUSPARSE failing LinearSolve.jl#827 (comment)EDIT: the spares jacobian additionaly fails because
OrdinaryDiffEq.jl/lib/OrdinaryDiffEqNonlinearSolve/src/initialize_dae.jl
Lines 387 to 390 in e038be1
does not work for sparse cuda matrices (this type of indexing is not implemented).
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.