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

Implement the time integration method used in DualSPHysics and add docs for time integration #716

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

efaulhaber
Copy link
Member

@efaulhaber efaulhaber commented Feb 11, 2025

Supersedes #693.
Closes #683.

Find the rendered docs for this PR here:
https://trixi-framework.github.io/TrixiParticles.jl/previews/PR716/

Copy link

codecov bot commented Feb 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.50%. Comparing base (6d04215) to head (5927b88).

Files with missing lines Patch % Lines
src/general/time_integration.jl 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #716      +/-   ##
==========================================
- Coverage   70.53%   70.50%   -0.03%     
==========================================
  Files          96       97       +1     
  Lines        5976     5978       +2     
==========================================
  Hits         4215     4215              
- Misses       1761     1763       +2     
Flag Coverage Δ
unit 70.50% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@efaulhaber efaulhaber requested review from svchb and LasNikas February 11, 2025 17:21
@efaulhaber efaulhaber self-assigned this Feb 12, 2025
Copy link
Collaborator

@svchb svchb left a comment

Choose a reason for hiding this comment

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

Update News.md

@efaulhaber efaulhaber requested a review from svchb February 20, 2025 09:37
Copy link
Collaborator

@LasNikas LasNikas left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks a lot

Can this only be used for WCSPH? It shouldn't be hard to add the EDAC scheme, right?
I mean, instead of evolving the density, we evolve the pressure.

using OrdinaryDiffEq
sol = solve(ode, Euler(),
dt=1.0,
save_everystep=false, callback=callbacks);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mention why we set save_everystep=false?

@efaulhaber
Copy link
Member Author

You can always use the original LeapfrogDriftKickDrift (which I recently added to OrdinaryDiffEq.jl and is available in the latest release) with EDAC, which treats pressure (or density for WCSPH) in the same way as the velocity. I haven't found an advantage of SymplecticPositionVerlet over LeapfrogDriftKickDrift yet.


### Features

- The symplectic time integration scheme from DualSPHysics was added (#716)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- The symplectic time integration scheme from DualSPHysics was added (#716)
- The symplectic time integration scheme based on DualSPHysics was added (#716)

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
- The symplectic time integration scheme from DualSPHysics was added (#716)
- The symplectic time integration scheme used in DualSPHysics was added (#716)

Better maybe?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also fine

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.

Time Integration page in the docs is empty
3 participants