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

GridTools/GridGenerator: add some functions to support dim-independent programming #9739

Merged
merged 1 commit into from Apr 4, 2020

Conversation

nfehn
Copy link
Contributor

@nfehn nfehn commented Mar 26, 2020

The functions rotate() and extrude_triangulation() are currently only implemented for dim=2 or 3. This PR allows to write code that can be compiled for dim=2,3.

@drwells
Copy link
Member

drwells commented Mar 26, 2020

The changes to rotate make sense but I'm not so sure about extrusion. What case did you find where we need an additional extrusion overload?

@nfehn
Copy link
Contributor Author

nfehn commented Mar 26, 2020

I get this error message

error: no matching function for call to extrude_triangulation(dealii::Triangulation<2, 2>&, unsigned int, const double&, dealii::Triangulation<2, 2>&)

@drwells
Copy link
Member

drwells commented Mar 26, 2020

Sure, but what does the call site look like?

@nfehn
Copy link
Contributor Author

nfehn commented Mar 26, 2020

something like this
Triangulation<2> tria_2d; Triangulation<dim> tria_dim; GridGenerator::extrude_triangulation(tria_2d, foo, bar, tria_dim);

@nfehn
Copy link
Contributor Author

nfehn commented Mar 30, 2020

@drwells Did I answer your questions? Please let me know if I still misunderstood something.

@drwells
Copy link
Member

drwells commented Mar 30, 2020

Sorry, I meant to get back to you on this. I think this is fine but we need to add documentation explaining the extrude is only available for 2D to 3D.

@tjhei
Copy link
Member

tjhei commented Mar 30, 2020

I think this is fine

I also think that it is a bit weird but it does help make user code much simpler without having to rely on constexpr if (my code that uses extrude() always contains specialization based on dimension because of this).

@drwells
Copy link
Member

drwells commented Apr 2, 2020

/rebuild

@@ -6164,7 +6164,6 @@ namespace GridGenerator
}



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 this line back in? We typically tend to keep three empty lines between function implementations for readability, see here: https://dealii.org/developer/doxygen/deal.II/CodingConventions.html (style issues, no 4). Similarly also between the new function and the old one below.

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 the empty lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants