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

Add tetrahedral grid #230

Merged
merged 55 commits into from
Dec 20, 2024
Merged

Add tetrahedral grid #230

merged 55 commits into from
Dec 20, 2024

Conversation

arlauwer
Copy link
Contributor

Description
Added the TetraMeshSpatialGrid class. This grid enables building a unique Delaunay tetrahedralization from input vertices, with a refinement option to increase quality of grid cells. This is all achieved using the open-source TetGen library. Vertices can be sampled from the medium or read from a text data file. Direct reading of tetrahedral grids is not currently supported.

Motivation
As described in Lauwers et al. (2024), adaptively constructed tetrahedral grids did not outperform existing grids in SKIRT. Nonetheless, a basic implementation of this grid can still be useful in SKIRT.

Tests
The previous iteration of this grid was extensively tested and benchmarked for accuracy. This implementation was also tested for accuracy and tested with a few edge cases, such as grids with a single tetrahedron or vertices resulting in a degenerate Delaunay tetrahedralization.

Problems

  • The documentation has a minor issue where Doxygen generates class references for the private TetraMeshSpatialGrid::Tetra and TetraMeshSpatialGrid::Face classes. These classes are declared inside the TetraMeshSpatialGrid.hpp
  • TetGen still displays many warnings when compiling.

arlauwer and others added 30 commits September 30, 2023 16:07
- changed DensityTreePolicy & TreePolicy consequently
- added extra contains & intersects to Box
cleaned up unused stuff from voronoi
started work on PathSegmentGenerator
added TetraMeshSpatialGrid options
added NeedsSubdivide as tetgen unsuitable
changed search tree to use vertices instead of centroids
(vertex intersection still not working)
only entry vertex intersection isn't working
small fixes to prepare TetraMeshSpatialGrid
added all Tetra classes
started work on refining Delaunay
marked potential oversight in Voro
`TetraMeshSnapshot::readAndClose` still needs to infer cell properties from vertex properties
@petercamps
Copy link
Contributor

With my recent commits I suppressed the warnings originating from the tetgen library code for various compilers (Clang, GCC, MSVC, Intel oneAPI). Furthermore I made some cosmetic updates to the TetraMeshSpatialGrid code, including:

  • always initialize local variables and data members at the point of declaration
  • replace global using std::array by custom type definitions (see "using directives and declarations" in https://skirt.ugent.be/root/_dev_coding.html
  • streamline the use of trailing zeroes and decimal point for floating point constants
  • replace explicit copying of vector<Vec> by the built-in assignment operator
  • move local variable declarations to right before where they are first used (in function storeTetrahedra)

@petercamps
Copy link
Contributor

Questions remaining:

  • At the end of setupSelfBefore(), mesh construction is aborted if there are fewer than 4 vertices. Rather than simply returning, I think the function should throw a FATALERROR.
  • Path construction is aborted if the originating point is outside of the convex hull. This means that sources outside of the convex hull and thus also those outside of the cuboidal grid domain are ignored. This seems unacceptable (e.g. the TRUST benchmark has a single point source, which is outside of the grid domain).

@arlauwer
Copy link
Contributor Author

arlauwer commented Dec 19, 2024

After discussion with @mbaes we have decided that simply adding 8 vertices in the corners of the grid bounding box would suffice for this basic implementation. The grid will now fully cover the simulation domain and sources outside the domain are possible again. The error for not enough vertices is no longer needed as there will always be at least 8 vertices.
Additionally, the grid now removes vertices that are too close to each other. This new functionality was also tested.

@petercamps petercamps merged commit 81d2eda into SKIRT:master Dec 20, 2024
6 checks passed
@arlauwer arlauwer deleted the tetra branch January 9, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants