-
Notifications
You must be signed in to change notification settings - Fork 4
Covariant Advection in DGMulti #111
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
base: main
Are you sure you want to change the base?
Covariant Advection in DGMulti #111
Conversation
benegee
left a comment
There was a problem hiding this 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) |
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this TODO?
There was a problem hiding this comment.
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.
| # 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. |
There was a problem hiding this comment.
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.
| local_values_threaded = [Trixi.allocate_nested_array(uEltype, nvars, (rd.Nq,), dg) | ||
| for _ in 1:Threads.nthreads()] |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
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.