PERK p2 - Githubissues

warisa-r commented 6 months ago

Hello,

this branch adds a new time integrator of second-order paired explicit Runge-Kutta method. It contains the time integrator, a polynomial optimizer for the method, and a new elixir employing the second-order P-ERK time integration scheme with 6 stages.

github-actions[bot] commented 6 months 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

[ ] The PR has a single goal that is clear from the PR title and/or description.
[ ] All code changes represent a single set of modifications that logically belong together.
[ ] No more than 500 lines of code are changed or there is no obvious way to split the PR into multiple PRs.

Code quality

[ ] The code can be understood easily.
[ ] Newly introduced names for variables etc. are self-descriptive and consistent with existing naming conventions.
[ ] There are no redundancies that can be removed by simple modularization/refactoring.
[ ] There are no leftover debug statements or commented code sections.
[ ] The code adheres to our conventions and style guide, and to the Julia guidelines.

Documentation

[ ] New functions and types are documented with a docstring or top-level comment.
[ ] Relevant publications are referenced in docstrings (see example for formatting).
[ ] Inline comments are used to document longer or unusual code sections.
[ ] Comments describe intent ("why?") and not just functionality ("what?").
[ ] If the PR introduces a significant change or new feature, it is documented in NEWS.md.

Testing

[ ] The PR passes all tests.
[ ] New or modified lines of code are covered by tests.
[ ] New or modified tests run in less then 10 seconds.

Performance

[ ] There are no type instabilities or memory allocations in performance-critical parts.
[ ] If the PR intent is to improve performance, before/after time measurements are posted in the PR.

Verification

[ ] The correctness of the code was verified using appropriate tests.
[ ] If new equations/methods are added, a convergence test has been run and the results are posted in the PR.

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

DanielDoehring commented 6 months ago

@warisa-r is the student researcher assisting me I mentioned in the last Trixi meeting. I encouraged her to open the PR herself.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 89.68610% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 75.43%. Comparing base (2dfde7f) to head (1b3d4f6).

Files	Patch %	Lines
...ation/paired_explicit_runge_kutta/methods_PERK2.jl	85.89%	23 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1871 +/- ## =========================================== - Coverage 96.30% 75.43% -20.88% =========================================== Files 439 441 +2 Lines 35749 35912 +163 =========================================== - Hits 34428 27087 -7341 - Misses 1321 8825 +7504 ``` | [Flag](https://app.codecov.io/gh/trixi-framework/Trixi.jl/pull/1871/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/1871/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=trixi-framework) | `75.43% <89.69%> (-20.88%)` | :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.

DanielDoehring commented 6 months ago

Concerning the allocations: This is no real type instability, but probably something related to the timing itself.

In particular, the allocations do not increase if the simulation is run for a longer time:

t = 1.0:

 ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
                             Trixi.jl                                      Time                    Allocations      
                                                                  ───────────────────────   ────────────────────────
                         Tot / % measured:                             336μs /  72.6%           43.3KiB /  83.7%    

 Section                                                  ncalls     time    %tot     avg     alloc    %tot      avg
 ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main loop                                                     1    152μs   62.5%   152μs   24.4KiB   67.2%  24.4KiB
   Paired Explicit Runge-Kutta ODE integration step           13   74.3μs   30.5%  5.72μs   7.34KiB   20.3%     578B
     rhs!                                                     78   60.5μs   24.8%   776ns   6.61KiB   18.2%    86.8B
       ~rhs!~                                                 78   30.0μs   12.3%   384ns   6.61KiB   18.2%    86.8B
       volume integral                                        78   14.8μs    6.1%   190ns     0.00B    0.0%    0.00B
       interface flux                                         78   3.95μs    1.6%  50.7ns     0.00B    0.0%    0.00B
       prolong2interfaces                                     78   2.62μs    1.1%  33.6ns     0.00B    0.0%    0.00B
       surface integral                                       78   2.03μs    0.8%  26.0ns     0.00B    0.0%    0.00B
       Jacobian                                               78   2.00μs    0.8%  25.7ns     0.00B    0.0%    0.00B
       reset ∂u/∂t                                            78   1.55μs    0.6%  19.8ns     0.00B    0.0%    0.00B
       prolong2boundaries                                     78   1.29μs    0.5%  16.6ns     0.00B    0.0%    0.00B
       boundary flux                                          78   1.19μs    0.5%  15.3ns     0.00B    0.0%    0.00B
       source terms                                           78   1.09μs    0.4%  14.0ns     0.00B    0.0%    0.00B
     ~Paired Explicit Runge-Kutta ODE integration step~       13   13.8μs    5.7%  1.06μs      752B    2.0%    57.8B
   analyze solution                                            1   73.5μs   30.2%  73.5μs   11.6KiB   31.9%  11.6KiB
   ~main loop~                                                 1   3.99μs    1.6%  3.99μs   5.45KiB   15.0%  5.45KiB
   calculate dt                                               13    562ns    0.2%  43.2ns     0.00B    0.0%    0.00B
 analyze solution                                              1   91.1μs   37.4%  91.1μs   11.9KiB   32.8%  11.9KiB
 calculate dt                                                  1    154ns    0.1%   154ns     0.00B    0.0%    0.00B
 ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────

t = 100:

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
                             Trixi.jl                                      Time                    Allocations      
                                                                  ───────────────────────   ────────────────────────
                         Tot / % measured:                            8.26ms /  98.5%            501KiB /  98.6%    

 Section                                                  ncalls     time    %tot     avg     alloc    %tot      avg
 ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main loop                                                     1   8.05ms   98.9%  8.05ms    482KiB   97.6%   482KiB
   Paired Explicit Runge-Kutta ODE integration step        1.28k   6.93ms   85.1%  5.41μs   7.34KiB    1.5%    5.88B
     rhs!                                                  7.68k   5.71ms   70.2%   744ns   6.61KiB    1.3%    0.88B
       ~rhs!~                                              7.68k   2.72ms   33.4%   354ns   6.61KiB    1.3%    0.88B
       volume integral                                     7.68k   1.48ms   18.2%   193ns     0.00B    0.0%    0.00B
       interface flux                                      7.68k    377μs    4.6%  49.1ns     0.00B    0.0%    0.00B
       prolong2interfaces                                  7.68k    253μs    3.1%  33.0ns     0.00B    0.0%    0.00B
       surface integral                                    7.68k    194μs    2.4%  25.3ns     0.00B    0.0%    0.00B
       Jacobian                                            7.68k    193μs    2.4%  25.1ns     0.00B    0.0%    0.00B
       reset ∂u/∂t                                         7.68k    150μs    1.8%  19.6ns     0.00B    0.0%    0.00B
       prolong2boundaries                                  7.68k    125μs    1.5%  16.3ns     0.00B    0.0%    0.00B
       boundary flux                                       7.68k    116μs    1.4%  15.1ns     0.00B    0.0%    0.00B
       source terms                                        7.68k    105μs    1.3%  13.6ns     0.00B    0.0%    0.00B
     ~Paired Explicit Runge-Kutta ODE integration step~    1.28k   1.21ms   14.9%   948ns      752B    0.1%    0.59B
   analyze solution                                           13    868μs   10.7%  66.8μs    153KiB   30.9%  11.7KiB
   ~main loop~                                                 1    203μs    2.5%   203μs    322KiB   65.2%   322KiB
   calculate dt                                            1.28k   51.7μs    0.6%  40.4ns     0.00B    0.0%    0.00B
 analyze solution                                              1   88.5μs    1.1%  88.5μs   11.9KiB    2.4%  11.9KiB
 calculate dt                                                  1    148ns    0.0%   148ns     0.00B    0.0%    0.00B
 ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────

DanielDoehring commented 6 months ago

For some reason the SummaryCallback causes the allocations causing the test to fail.

This is observed also for the other custom integrators, see

https://github.com/trixi-framework/Trixi.jl/blob/2dfde7faf3cc74f066d86148ae6c99ed9e58fa79/test/test_tree_2d_advection.jl#L153-L166

and the other tests.

DanielDoehring commented 6 months ago

Not sure if I can add a version for DelimitedFiles as I get for DelimitedFiles = "1.9.0"

  ERROR: LoadError: Unsatisfiable requirements detected for package DelimitedFiles [8bb1440f]:
 DelimitedFiles [8bb1440f] log:
 ├─possible versions are: 1.9.0-1.9.1 or uninstalled
 └─restricted to versions 1.8.5-1.8 by Trixi [a7f1ee26] — no versions lef

and for DelimitedFiles=1.8.5 (for which no release seems to exist)

  ERROR: LoadError: Unsatisfiable requirements detected for package DelimitedFiles [8bb1440f]:
 DelimitedFiles [8bb1440f] log:
 ├─possible versions are: 1.8.5 or uninstalled (package in sysimage!)
 └─restricted to versions 1.9.0-1 by Trixi [a7f1ee26] — no versions left

@JoshuaLampert

JoshuaLampert commented 6 months ago

Yes, for the Downgrade action that's to be expected as DelimitedFiles is a stdlib. It should be added to the list of skipped libraries. The compat could then be DelimitedFiles = "1.9.0". You could also try DelimitedFiles = "1".

JoshuaLampert commented 6 months ago

A bit of context for why we need the compat: https://discourse.julialang.org/t/psa-compat-requirements-in-the-general-registry-are-changing/104958 and https://github.com/trixi-framework/Trixi.jl/pull/1711.

DanielDoehring commented 6 months ago

You could also try DelimitedFiles = "1".

That seems to work, thanks!

warisa-r commented 5 months ago

Close this one for now. Working on making Convex and ECOS extensions rather than direct dependencies.

trixi-framework / Trixi.jl

PERK p2 #1871

Review checklist

Purpose and scope

Code quality

Documentation

Testing

Performance

Verification

Codecov Report