trixi-framework / Trixi.jl

Trixi.jl: Adaptive high-order numerical simulations of conservation laws in Julia
https://trixi-framework.github.io/Trixi.jl
MIT License
505 stars 98 forks source link

Switch `DGMulti` to `Matrix{<:SVector}` solution storage #1952

Open jlchan opened 1 month ago

jlchan commented 1 month ago

Also bumps StartUpDG.jl compat to 1.0.

github-actions[bot] commented 1 month ago

Review checklist

This checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging.

Purpose and scope

Code quality

Documentation

Testing

Performance

Verification

Created with :heart: by the Trixi.jl community.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.03%. Comparing base (c2513e2) to head (c7b38bb).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1952 +/- ## =========================================== - Coverage 96.09% 82.03% -14.06% =========================================== Files 457 457 Lines 36734 36661 -73 =========================================== - Hits 35298 30074 -5224 - Misses 1436 6587 +5151 ``` | [Flag](https://app.codecov.io/gh/trixi-framework/Trixi.jl/pull/1952/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trixi-framework) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/trixi-framework/Trixi.jl/pull/1952/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trixi-framework) | `82.03% <100.00%> (-14.06%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trixi-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jlchan commented 1 month ago

I am running into issues due to Polyester.@batch turning arrays into StrideArraysCore.PtrArrays". For a Navier-Stokes solve, this dispatches to Octavian.matmul! via https://github.com/JuliaSIMD/StrideArrays.jl/blob/d31fb6cf28b0374162202da8c81694f2b704e0f3/src/blas.jl#L6.

For now, I'll just focus on the single-threaded case, but dealing with multi-threading will require figuring out if Polyester and StrideArrays will continue after stopping support of LoopVec.jl and Octavian.jl in Julia v1.11 (cc @chriselrod)

chriselrod commented 1 month ago

For a Navier-Stokes solve, this dispatches to Octavian.matmul! via https://github.com/JuliaSIMD/StrideArrays.jl/blob/d31fb6cf28b0374162202da8c81694f2b704e0f3/src/blas.jl#L6.

Feel free to PR it to drop Octavian as a dependency.

For now, I'll just focus on the single-threaded case, but dealing with multi-threading will require figuring out if Polyester and StrideArrays will continue after stopping support of LoopVec.jl and Octavian.jl in Julia v1.11

Regardless of the official status, you don't have to rush to make changes. There are no test failures on 1.11.

jlchan commented 1 month ago

For a Navier-Stokes solve, this dispatches to Octavian.matmul! via https://github.com/JuliaSIMD/StrideArrays.jl/blob/d31fb6cf28b0374162202da8c81694f2b704e0f3/src/blas.jl#L6.

Feel free to PR it to drop Octavian as a dependency.

For now, I'll just focus on the single-threaded case, but dealing with multi-threading will require figuring out if Polyester and StrideArrays will continue after stopping support of LoopVec.jl and Octavian.jl in Julia v1.11

Regardless of the official status, you don't have to rush to make changes. There are no test failures on 1.11.

Thanks - I removed the LinearAlgebra.mul! overloads in https://github.com/jlchan/StrideArrays.jl/blob/d31fb6cf28b0374162202da8c81694f2b704e0f3/src/blas.jl#L2-L36. It runs, but the solution gets NaNs. When I switch @batch to Threads.@threads, I get an error that seems related to @turbo:

ERROR: TaskFailedException

    nested task error: UndefRefError: access to undefined reference

so I think I'm going to have to dig a little deeper beyond removing Octavian dependencies to figure out what's going wrong...