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 7 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 (78a23f3) to head (429b0a1).

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
Comment on lines +13 to +14
This approach allows us to use specialized time integration methods that do not work with
general `ODEProblem`s.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Like for example?

### [Leapfrog kick-drift-kick](@id kick_drift_kick)

The kick-drift-kick scheme of the leapfrog method, updating positions ``u``
and velocities ``v`` with the functions ``\operatorname{kick}`` and ``\operatorname{drift}``,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Function kick below is (u,v, t) here it is (u,t) this difference should be explained

### Symplectic position Verlet

When the density is integrated (with [`ContinuityDensity`](@ref)), the density is appended
to ``v`` as additional dimension, so all previously mentioned schemes treat the density
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
to ``v`` as additional dimension, so all previously mentioned schemes treat the density
to ``v`` as an additional dimension, so all previously mentioned schemes treat the density

The SPH code [DualSPHysics](https://github.com/DualSPHysics/DualSPHysics) implements
a variation of the drift-kick-drift scheme where the density is updated separately.
In the following, we will call the derivative of the density ``R(v, u, t)``,
even though it is actually included in the ``\operatorname{kick}`` as additional dimension.
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
even though it is actually included in the ``\operatorname{kick}`` as additional dimension.
even though it is actually included in the ``\operatorname{kick}`` as an additional dimension.

Comment on lines +84 to +85
Note that the method `VelocityVerlet` does not work with TrixiParticles.jl for the same and
other additional reasons.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Weird jump. Please include the explanation in the previous text. Also redundant because of the warning below.

Comment on lines +49 to +50
uprev, uprev2, f, t,
dt, reltol, p, calck,
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
uprev, uprev2, f, t,
dt, reltol, p, calck,
uprev, uprev2, f, t, dt, reltol, p, calck,

ext/TrixiParticlesOrdinaryDiffEqExt.jl Show resolved Hide resolved
tTypeNoUnits}
# We only use inplace functions in TrixiParticles, so there is no point
# in implementing the non-inplace version.
error("`SymplecticPositionVerlet` only supports inplace functions")
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
error("`SymplecticPositionVerlet` only supports inplace functions")
error("`SymplecticPositionVerlet` supports only in-place functions.")

Comment on lines +83 to +84
# verify_f2(integrator.f.f2, integrator.k[2].x[2], duprev, uprev, integrator.p,
# integrator.t, integrator, cache)
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
# verify_f2(integrator.f.f2, integrator.k[2].x[2], duprev, uprev, integrator.p,
# integrator.t, integrator, cache)

@@ -171,6 +171,13 @@ @Article{Bonet1999
publisher = {Elsevier BV},
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also cite a DualSPHysics paper in which this is explained.

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
2 participants