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

Feature: t8code as meshing backend #1426

Merged
merged 87 commits into from
Jul 26, 2023
Merged

Feature: t8code as meshing backend #1426

merged 87 commits into from
Jul 26, 2023

Conversation

jmark
Copy link
Contributor

@jmark jmark commented May 5, 2023

This is the first of a series of probably many PRs to provide full t8code meshing support for Trixi.jl. As of now, only Quads (in 2D) is supported. The datastructures on the Trixi-side are almost identical to the p4est mesh. Hence, the DG solver implementation dgsem_p4est is now shared between the two meshing backends. Preliminary tests show that both backends give exact same convergence results up to floating point precisions as is proven by identical unit tests.

@jmark
Copy link
Contributor Author

jmark commented May 5, 2023

The code does not compile via Github CI since T8code.jl is not registered as an offical package yet.

@jmark jmark added the enhancement New feature or request label May 5, 2023
@jmark jmark self-assigned this May 5, 2023
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

This is a great step forward, thanks a lot! Please find below some comments after a quick first look at the code. I think there are also some places where some additional comments explaining the "why and what" could be helpful.

Project.toml Show resolved Hide resolved
src/Trixi.jl Show resolved Hide resolved
src/auxiliary/auxiliary.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
@jmark jmark marked this pull request as ready for review May 23, 2023 07:12
@jlchan jlchan self-requested a review May 23, 2023 14:26
@jmark jmark marked this pull request as draft May 23, 2023 14:31
@jmark
Copy link
Contributor Author

jmark commented May 23, 2023

Some thoughts to consider:

  • T8codeMesh povides same functionality in terms of the DGSEM solver as P4estMesh.
  • Tests are identical to the ones for p4est_2d giving exact same results.
  • I tried to reuse as much of the code implemented for the P4estMesh as possible. The datastructures for the caches are exactly the same. Question: How to get rid of T8codeElementContainer, T8codeInterfaceContainer, ... in the cache implementation but still utilize multi dispatch?
  • In this PR the backend only supports 2D quads in serial or threaded. 3D hexs and MPI are planned for later PRs.
  • Addtional functionality comes from the t8code library itself, such as advanced geometry, complex meshing, etc., which offers more features than p4est provides.
  • Other element types or hybrid meshes will be implemented via the DGMulti solver. This means a completly different approach on how t8code interacts with Trixi. Code from this PRs and related will not be useful.

@jmark jmark assigned jlchan, sloede and ranocha and unassigned jmark May 23, 2023
@jmark jmark requested a review from ranocha May 23, 2023 14:50
@jmark jmark assigned jlchan, sloede and ranocha and unassigned jlchan, sloede and ranocha May 23, 2023
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot for our work! Please find below some first comments. I will finish the review later (hopefully this week).

src/auxiliary/auxiliary.jl Outdated Show resolved Hide resolved
src/callbacks_step/analysis.jl Outdated Show resolved Hide resolved
src/solvers/dgsem_p4est/containers_2d.jl Outdated Show resolved Hide resolved
test/test_t8code_2d.jl Outdated Show resolved Hide resolved
test/test_t8code_2d.jl Outdated Show resolved Hide resolved
@jmark
Copy link
Contributor Author

jmark commented Jul 25, 2023

In df94655, you deleted a finalizer function. Does this mean that we leave garbage around? If so, could you check inside the call whether MPi is already finalized instead?

I opened an issue explaining the problem: #1585.

Proposed a solution for #1585. See discussion there.

Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot! We are pretty close to finishing this. Thanks you your patience.

examples/t8code_2d_dgsem/elixir_advection_basic.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
src/meshes/t8code_mesh.jl Outdated Show resolved Hide resolved
src/auxiliary/t8code.jl Outdated Show resolved Hide resolved
ranocha
ranocha previously approved these changes Jul 25, 2023
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@ranocha ranocha enabled auto-merge (squash) July 26, 2023 03:56
@jmark jmark assigned ranocha and unassigned jmark Jul 26, 2023
@jmark
Copy link
Contributor Author

jmark commented Jul 26, 2023

Made the sc_finalize logic optional. See #1585 (comment) .

@ranocha ranocha merged commit fe0e78c into main Jul 26, 2023
30 of 31 checks passed
@ranocha ranocha deleted the feature-t8code branch July 26, 2023 08:47
Copy link
Member

@sloede sloede left a comment

Choose a reason for hiding this comment

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

Great work! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants