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

Create a Triangulation<3> from a Triangulation<2,3> #13767

Merged

Conversation

fdrmrc
Copy link
Contributor

@fdrmrc fdrmrc commented May 20, 2022

This creates a Triangulation<3> out of a Triangulation<2,3>, filling the latter with tetrahedrons. Named parameters can be used to control the size and the quality of the final mesh.
I put this into GridTools, but I'm happy to move this somewhere else if you think it's not the right namespace/place.

The .cc test is just filling an hypersphere, but one could use this after boolean operations (#13717) , giving the capability of meshing things like the following, where the union of two spheres is computed and meshed inside
boolean_spheres_union

@luca-heltai
Copy link
Member

I think this should go in grid_geneerator.h, next to its "inverse", i.e., the function that extracts a boundary mesh from a Triangulation.

Copy link
Member

@luca-heltai luca-heltai left a comment

Choose a reason for hiding this comment

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

Besides moving it to grid_generator.h, I think the documentation should be filled with the information about the additional parameters.

include/deal.II/grid/grid_tools.h Outdated Show resolved Hide resolved
tests/cgal/cgal_triangulation_07.cc Outdated Show resolved Hide resolved
@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch 3 times, most recently from 8d4108a to 4b1e1f4 Compare May 22, 2022 21:09
Copy link
Member

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

Very nice!

void
surface_mesh_to_volumetric_mesh(const Triangulation<2, 3> &surface_tria,
Triangulation<3> & vol_tria,
Args... cgal_args);
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would prefer to introduce an AdditionalData.

Copy link
Member

Choose a reason for hiding this comment

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

Can we move #ifdef DEAL_II_WITH_CGAL within the function and assert it there?

Comment on lines 941 to 947
#ifdef DEAL_II_WITH_CGAL
template <typename... Args>
void
surface_mesh_to_volumetric_mesh(const Triangulation<2, 3> &surface_tria,
Triangulation<3> & vol_tria,
Args... cgal_args);
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Why is this here? Isn't this the same as above?

@fdrmrc
Copy link
Contributor Author

fdrmrc commented May 22, 2022

@peterrum I pushed right after you wrote the review!

@peterrum
Copy link
Member

One question: the original surface mesh determines the cells of the volume mesh on the surface or are the cells unrelated?

@fdrmrc
Copy link
Contributor Author

fdrmrc commented May 22, 2022

One question: the original surface mesh determines the cells of the volume mesh on the surface or are the cells unrelated?

They're currently unrelated. The use case I had in mind is like the one in the test: one just starts with a deal.II tria describing the surface and fills it. Another use case is the one with boolean operations I wrote in first message. However, it shouldn't be a problem to record the topological information about the first deal.II surface tria and use it to create the volumetric one.

@luca-heltai luca-heltai changed the title Convert Triangulation<2,3> to Triangulation<3> Create a Triangulation<3> from a Triangulation<2,3> May 25, 2022
@luca-heltai luca-heltai added this to the Release 9.4 milestone May 27, 2022
Comment on lines 1789 to 1793
template <typename... Args>
void
surface_mesh_to_volumetric_mesh(const Triangulation<2, 3> &surface_tria,
Triangulation<3> & vol_tria,
Args... cgal_args);
Copy link
Member

Choose a reason for hiding this comment

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

I will object to this on the same grounds as for the other patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. As soon as the other patch is okay, I can update this!

@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch 2 times, most recently from ab71efb to b5418bc Compare June 6, 2022 14:48
@fdrmrc
Copy link
Contributor Author

fdrmrc commented Jun 6, 2022

So done, this should be good to go now.
Since the function is not templated anymore, I've moved it inside grid_generator.cc.

Also, I took the liberty to move the two utility functions that convert points back and forth from deal.II to CGAL in a separate header, fixing some issues with headers inclusions I hadn't noticed before.

Copy link
Member

@luca-heltai luca-heltai left a comment

Choose a reason for hiding this comment

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

Minor change required: add an implementation stub that throws ExcNotImplemented if cgal is not found.

CGALWrappers::cgal_triangulation_to_dealii_triangulation(cgal_triangulation,
vol_tria);
}
# endif
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a default implementation that throws an error saying that the function needs CGAL to be installed?

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

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

Looks good, apart from some adaptations to the comments.


/**
* Create a deal.II Triangulation<3> out of a deal.II Triangulation<2,3>
* by filling it with tetrahedrons.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* by filling it with tetrahedrons.
* by filling it with tetrahedra.

Comment on lines 1832 to 1834
* The optional variable arguments @p cgal_args can be used to pass additional
* arguments to the CGAL::Mesh_criteria_3 class (see
* https://doc.cgal.org/latest/Mesh_3/index.html) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please adapt this and the text below, as we have now introduced AdditionalData.

@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch 4 times, most recently from 1df12e7 to be72338 Compare June 7, 2022 16:49
@fdrmrc
Copy link
Contributor Author

fdrmrc commented Jun 7, 2022

I'm addressing Luca's comment, please don't merge

@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch from be72338 to 1371aac Compare June 7, 2022 19:39
@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch 3 times, most recently from bb17e55 to 8141e31 Compare June 8, 2022 09:42
@fdrmrc
Copy link
Contributor Author

fdrmrc commented Jun 8, 2022

I rebased this PR onto the smaller ones #13929 and #13932, so that here there's effectively only the implementation of surface_tria_to_volumetric().

So only d1e69d2 is the relevant commit

@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch 5 times, most recently from d1e69d2 to 2337fe4 Compare June 10, 2022 17:12
@fdrmrc
Copy link
Contributor Author

fdrmrc commented Jun 10, 2022

Rebased on top of master.

@fdrmrc fdrmrc force-pushed the cgal-surface_tria_to_volumetric branch from 2337fe4 to 8d243fd Compare June 11, 2022 09:46
@fdrmrc
Copy link
Contributor Author

fdrmrc commented Jun 11, 2022

The implementation of the present function has been moved in grid_generator_cgal.cc, following #13954

@kronbichler kronbichler merged commit 990806d into dealii:master Jun 11, 2022
CGAL Support automation moved this from In progress to Done Jun 11, 2022
mkghadban pushed a commit to OpenFCST/dealii that referenced this pull request Sep 8, 2022
…_volumetric

Create a Triangulation<3> from a Triangulation<2,3>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants