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

Paur task1 #18

Merged
merged 11 commits into from Jun 29, 2021
Merged

Paur task1 #18

merged 11 commits into from Jun 29, 2021

Conversation

amartinhuertas
Copy link
Member

@amartinhuertas amartinhuertas commented Jun 29, 2021

To describe ... @ericneiva @fverdugo @paurierap involved

Related to #16

paurierap and others added 10 commits June 8, 2021 13:17
Clean-up of the module and first tests.
Visualize a Grid() or UnstructuredGrid() using mesh and wireframe from Makie.
Add nodaldata and colormap. Send .DS_Store to .gitignore.
Extension of wireframe function to fit quad meshes. Addition of colormap and colorbar in the tests.
Added a couple of lines in the conversion in 3D: the connectivity cns is converted via SimplexFace{D+1,Type} instead of NgonFace{D+1,Type} since the latter creates quads in 3D. Also, Makie.mesh does not support the type tetrahedron, so the grid is converted with normal_mesh.
@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2021

Codecov Report

❗ No coverage uploaded for pull request base (gsoc2021-master@176550d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##             gsoc2021-master      #18   +/-   ##
==================================================
  Coverage                   ?   94.11%           
==================================================
  Files                      ?        1           
  Lines                      ?       34           
  Branches                   ?        0           
==================================================
  Hits                       ?       32           
  Misses                     ?        2           
  Partials                   ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 176550d...c4097f4. Read the comment docs.

@fverdugo fverdugo marked this pull request as draft June 29, 2021 08:04
@fverdugo fverdugo marked this pull request as ready for review June 29, 2021 08:45
@fverdugo
Copy link
Member

@paurierap I have refactored the conversions significanly. The goal of this refactoring is:

  • Avoid type-piracy. By defining GeometryBasiscs.Mesh(::Grid) we are doing so-called type piracy since neither Mesh nor Grid are defined in this package. I have used another function name to fix this: to_plot_mesh so that grid |> to_plot_mesh returns the mesh ready to be visualized.
  • I have split the high-level conversion to_plot_mesh into smaller functions to_boundary_grid, to_mesh, and to_simplex_grid. These lower-level functions do only one thing. In the previous code, some functions did several things, which was less modular.
  • With these lower-level functions, the high-level conversion is much more readable than before:
function to_plot_mesh(grid::UnstructuredGrid)
  if num_cell_dims(grid) == 3
    grid |> to_boundary_grid |> to_simplex_grid |> to_mesh |> GeometryBasics.normal_mesh
  else
    grid |> to_simplex_grid |> to_mesh
  end
end

Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

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

This compleates all tasks in #16.

@fverdugo fverdugo merged commit 3ddfea9 into gsoc2021-master Jun 29, 2021
@fverdugo fverdugo deleted the paur-task1 branch June 29, 2021 09:03
@fverdugo
Copy link
Member

Merged!

@paurierap I have deleted the paur-task1 branch.

Plase create a new branch paur-task2 from gsoc2021-master to start working on the task described here #17

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.

None yet

4 participants