Skip to content

Conversation

@PhilBaa
Copy link

@PhilBaa PhilBaa commented Sep 4, 2025

This PR adds a covariant solver for DGMulti and a function constructing an icosahedral DGMultiMesh using prism elements, which are used in the new elixir elixir_spherical_advection_covariant_prismed_icosahedron.jl.

Copy link
Collaborator

@benegee benegee left a comment

Choose a reason for hiding this comment

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

I did an initial, very superficial review, because honestly I do not know much about the math and the DGMulti code you used.

# OrdinaryDiffEq's `solve` method evolves the solution in time and executes the passed
# callbacks
sol = solve(ode, CarpenterKennedy2N54(williamson_condition = false),
dt = 1.0, save_everystep = true, 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.

You could use ode_default_options() here.

@unpack u_values, aux_quad_values = cache

# interpolate u, du to quadrature points
du_values = similar(u_values) # TODO: DGMulti. Can we move this to the analysis cache somehow?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a general TODO that is addressed somewhere else?

Copy link
Author

Choose a reason for hiding this comment

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

I copied this over from the corresponding Trixi function. I will add a note pointing to Trixi somehow

end
aux_values[i, element] = SVector{n_aux}(aux_node)
end
# TODO: implement Christoffel symbols
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about this TODO?

Copy link
Author

Choose a reason for hiding this comment

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

Christoffel values are not yet used for this PR, so I left them out, because they can't be tested for now.

Comment on lines +1 to +4
# Construct a DGMultiMesh consisting of prismatic elements based on a refined icosahedron
# As of now this 3D mesh only serves to simulate 2D covariant equations on the sphere, with
# the spherical shell thus only consisting of a single layer of prismatic elements extruded
# from the triangular faces of the icosahedron.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the described "abuse" of the 3D mesh temporary to try out things? I would be absolutely fine with this.
If it is supposed to become permanent at some point, some comments in the code could be helpful for anyone trying to implement a similar construction.

Comment on lines +33 to +34
local_values_threaded = [Trixi.allocate_nested_array(uEltype, nvars, (rd.Nq,), dg)
for _ in 1:Threads.nthreads()]
Copy link
Collaborator

@benegee benegee Oct 28, 2025

Choose a reason for hiding this comment

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

This pattern is currently under discussion because, IIRC, it will not work with Julia 1.12 anymore.

@reexport import Trixi: waterheight

# DGMulti solvers
using StartUpDG: RefElemData, MeshData, AbstractElemShape
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are all of these needed in this PR, or maybe in some upcoming PR?

@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 0% with 258 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.27%. Comparing base (88acd71) to head (55c0301).

Files with missing lines Patch % Lines
src/meshes/dgmulti_prismed_sphere.jl 0.00% 55 Missing ⚠️
src/solvers/dgmulti/dg_manifold_covariant.jl 0.00% 55 Missing ⚠️
...c/solvers/dgmulti/containers_manifold_covariant.jl 0.00% 37 Missing ⚠️
src/callbacks_step/analysis_covariant.jl 0.00% 27 Missing ⚠️
src/solvers/dgmulti/dg.jl 0.00% 26 Missing ⚠️
src/callbacks_step/stepsize_dg2d.jl 0.00% 17 Missing ⚠️
..._step/save_solution_2d_manifold_in_3d_cartesian.jl 0.00% 14 Missing ⚠️
src/equations/covariant_advection.jl 0.00% 12 Missing ⚠️
src/callbacks_step/save_solution_covariant.jl 0.00% 10 Missing ⚠️
...semidiscretization_hyperbolic_2d_manifold_in_3d.jl 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   97.30%   91.27%   -6.04%     
==========================================
  Files          30       34       +4     
  Lines        3903     4161     +258     
==========================================
  Hits         3798     3798              
- Misses        105      363     +258     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants