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

Some updates for mesh generation #3275

Merged
merged 44 commits into from
Sep 12, 2024

Conversation

schnellerhase
Copy link
Contributor

While reading up on the mesh generation I had quite a hard time to understand what was going on in generation.h. This contains some tidying up of the code and relocation of input checks to more central positions.

What this PR does not change but should be done in the future is (if PR gets accepted I'll make this a new issue)

  • unify interface of box based mesh generation, i.e. combine create_interval, create_rectangle and create_box
    • the checks for valid parameters can then be performed in one central location
    • the geometries that are created are already in 3D space, but we do not make use of this. For example an interval mesh is currently only to be generated along the $x$-axis and not along an arbitrary line segment in 3D space. Same holds for the rectangle interface. Changing this, also allows for common data types, i.e. we will no longer need to handle point types of different dimensions.
  • extend testing -> currently a lot of the provided functionality is not tested thoroughly .

// Create tetrahedra
for (std::int64_t i = range_c[0]; i < range_c[1]; ++i)
{
const std::int64_t iz = i / (nx * ny);
Copy link
Member

Choose a reason for hiding this comment

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

We usually dont use const assignment for integers. Maybe @garth-wells can comment?

Copy link
Member

Choose a reason for hiding this comment

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

House style is we don't normally use const for owned variables inside functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So remove const in these cases? They were marked const before this PR.

cpp/dolfinx/mesh/generation.h Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
// Create tetrahedra
for (std::int64_t i = range_c[0]; i < range_c[1]; ++i)
{
const std::int64_t iz = i / (nx * ny);
Copy link
Member

Choose a reason for hiding this comment

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

House style is we don't normally use const for owned variables inside functions.

cpp/dolfinx/mesh/generation.h Outdated Show resolved Hide resolved
@schnellerhase
Copy link
Contributor Author

schnellerhase commented Jul 10, 2024

Depends on #3293

@mscroggs mscroggs added this to the 0.9.0 milestone Sep 3, 2024
@schnellerhase schnellerhase force-pushed the create_rectangle branch 3 times, most recently from 1109985 to 0737629 Compare September 11, 2024 20:18
schnellerhase and others added 15 commits September 11, 2024 22:19
* Fix cmake

* Implement a one dimensional refinement routine

* Change to assert

* Switch to ghost mode by user choice

* Switch naming from 'edges' to 'cells'

* More 'edge' -> 'cell'

* Add parameter of intervals and explicit data type

* Bug fix in test

* Remove vector<bool> comment

* Capitalization of comments

* Split -> refined

* Doc string

* Fix edge confusion

* Switch to ghost mode default shared_facet

* Fix float comparison

* Fix typo

* Fix type

* Remove auto

* Add brackets for readability
* Rename interval refinement test file

Pytest discovery previously ignored the interval test case, since it did not match the expected `test_` naming scheme

* Fix: implicit ghost_mode of interval mesh made explicit

* Fix: Bug for parallel interval refinement

Caused by assumption that ghost cells are refined on the process as well, which is not the case -> job of remote process that owns it
@schnellerhase
Copy link
Contributor Author

Fixed the wrong git setup in the initial commits.

@garth-wells garth-wells added this pull request to the merge queue Sep 12, 2024
Merged via the queue into FEniCS:main with commit de390e4 Sep 12, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants