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

p4est datastructures #780

Open
wants to merge 146 commits into
base: master
Choose a base branch
from
Open

p4est datastructures #780

wants to merge 146 commits into from

Conversation

koehlerson
Copy link
Member

@koehlerson koehlerson commented Jul 31, 2023

to be discussed at FerriteCon 23, register now: https://ferrite-fem.github.io/FerriteCon/

TODO

  • p4est types
    • OctantBWG morton index encoded octant in 2 and 3D
    • OctreeBWG linear tree with homogeneous OctantBWG leaves
    • ForestBWG forest of homogeneous OctreeBWG
  • Octant Operations
  • Octree operations
    • balance intertree
    • balance intratree
    • coarsen
    • refine
  • materializing a Ferrite.Grid for unstructured meshes
  • hanging nodes
  • error estimator interface
    • ZZ type error estimator slow version (probably working in docs/srcs/literate-tutorials/adaptivity.jl
  • Docs
    • Docs to the p4est types and how they interplay
    • Docs to the yet to be defined interface
    • quick and dirty implementation of elasticity docs/src/literate-tutorials/adaptivity.jl with (almost?maybe?) working but slow ZZ error estimator
    • manufactured solution example for heat equation at docs/src/literate-tutorials/heat-adaptivity.jl
    • adjust quick and dirty docs/src/literate-tutorials/adaptivity.jl to a proper tutorial
  • More debug statements to track down issues
  • multiple neighbors attached to edge and vertex
  • rotation test in 3D

Followup PRs

  • DofHandler integration
  • Transfer of solution vector between dof handlers
  • make it work for arbitrary polynomial order
  • error estimation
    • faster local ZZ error estimator
    • Kelly error estimator
  • iterators
    • cell
    • face
    • edge
    • vertex
  • conformization

Copy link
Member

@KnutAM KnutAM left a comment

Choose a reason for hiding this comment

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

Took a quick look with some nitty-gritty picks following the Slack-request. First of all: very nice effort, I think putting this work into devdocs is really helpful for the project!

I feel like I understand the indexing part well with the devdocs, but it is always hard to judge before having to actually work with it if it is enough (or too much).

What is not clear to me is how the reference coordinate system is related to the real geometry. But perhaps that will become clear as more regular docs are added?

docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
The basic idea is that each Octant (in 3D) or quadrant (in 2D) can be encoded by 2 quantities

- the level `l`
- the lower left (front) coordinates `xyz`
Copy link
Member

Choose a reason for hiding this comment

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

Here I got a bit confused since we change meaning of "lower" and "front" going 2d to 3d. Should it say something like "the coordinates of the vertex with the smallest coordinates" (not sure how to describe the best, from the restriction would "the coordinates of the vertex closest to the origin" work?)

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean with changing the meaning by going 2d to 3d? Do you mean that Ferrite does this? I think the problem is that you need to define where the origin is and they decided for the lower left front coordinate (without rotation)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, in Ferrite and 2d, bottom/top refers to the y-coordinate, but in 3d bottom/top refers to the z-coordinate (and front/back to the y-coordinate). I was confused because I first thought front would be the largest coordinate value (but it isn't), so it is correct (and logical) the way it is defined. But I find that refering to what is closest to the orgin / has the smallest coordinate value is easier, since it doesn't require knowing what is defined as front/back, left/right etc.

julia> grid = generate_grid(Hexahedron, (1,1,1));

julia> x = getcoordinates(grid, 1);

julia> fv = FaceValues(FaceQuadratureRule{RefHexahedron}(1), Lagrange{RefHexahedron,1}());

julia> for (key, face) in grid.facesets
           reinit!(fv, x, first(face)[2])
           println(key, ": ", spatial_coordinate(fv, 1, x))
       end
left: [-1.0, 0.0, 0.0]
bottom: [0.0, 0.0, -1.0]
right: [1.0, 0.0, 0.0]
back: [0.0, 1.0, 0.0]
top: [0.0, 0.0, 1.0]
front: [0.0, -1.0, 0.0]

julia> grid = generate_grid(Quadrilateral, (1,1));

julia> x = getcoordinates(grid, 1);

julia> fv = FaceValues(FaceQuadratureRule{RefQuadrilateral}(1), Lagrange{RefQuadrilateral,1}());

julia> for (key, face) in grid.facesets
           reinit!(fv, x, first(face)[2])
           println(key, ": ", spatial_coordinate(fv, 1, x))
       end
left: [-1.0, 0.0]
bottom: [0.0, -1.0]
right: [1.0, 0.0]
top: [0.0, 1.0]

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I see. I think it's just two different ways of attaching meaning to "lower", "left" etc, where p4est seems to be strict with the z-order logic. AFAIU the problem with defining the origin based on the lowest coordinate is that it would be a circular definition. You need some anchor point to define the coordinate system. The anchor point you choose will inevitably have the lowest coordinates.

docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
docs/src/devdocs/AMR.md Show resolved Hide resolved
docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved
docs/src/devdocs/AMR.md Outdated Show resolved Hide resolved

The current implementation of an octant looks currently like this:
```julia
struct OctantBWG{dim, N, T} <: AbstractCell{RefHypercube{dim}}
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
struct OctantBWG{dim, N, T} <: AbstractCell{RefHypercube{dim}}
struct OctantBWG{dim, N, T <: Integer} <: AbstractCell{RefHypercube{dim}}

And is this an ok restriction? Would make the discreteness clearer IMO

@koehlerson
Copy link
Member Author

koehlerson commented May 10, 2024

Took a quick look with some nitty-gritty picks following the Slack-request. First of all: very nice effort, I think putting this work into devdocs is really helpful for the project!

I feel like I understand the indexing part well with the devdocs, but it is always hard to judge before having to actually work with it if it is enough (or too much).

What is not clear to me is how the reference coordinate system is related to the real geometry. But perhaps that will become clear as more regular docs are added?

Thanks! I'll try to incorporate the suggestions as soon as I'm back at my laptop. Regarding the octree coordinate: you have to imagine that each octree spans a box with finite size where the root fills this box. In there, a coordinate system begins at the lower left front coordinate. This is just convention by p4est. You could also start somewhere else and adjust the rotation definitions accordingly. There exists a mapping between the physical coordinate and this new octree coordinate. The transformation from octree coordinates to physical ones is in transform_pointBWG

Co-authored-by: Kristoffer Carlsson <kcarlsson89@gmail.com>
Comment on lines 52 to 55
function transform_coordinates!(g::NonConformingGrid, f::Function)
replace!(n -> Node(f(get_node_coordinate(n))), g.nodes)
return g
end
Copy link
Member

Choose a reason for hiding this comment

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

Should no longer be needed after #914

@koehlerson
Copy link
Member Author

I'm currently merging master and the definitions of p4est what a face is mismatch now Ferrite's definition. So what should we do? I'm inclined to follow the p4est stuff internally, as it makes it easier to align the implementation with the paper algorithms.

@KnutAM
Copy link
Member

KnutAM commented May 21, 2024

inclined to follow the p4est stuff internally

Maybe use p4est_face and document that this corresponds to Ferrite.facet?

On the other hand, the advantage of facet is that it "looks" almost like face:). So if you only need to work with facets, I don't think that will be too confusing if it is mentioned clearly that p4est use face for what we call facet...

src/Dofs/DofHandler.jl Outdated Show resolved Hide resolved
@koehlerson
Copy link
Member Author

koehlerson commented May 23, 2024

inclined to follow the p4est stuff internally

Maybe use p4est_face and document that this corresponds to Ferrite.facet?

On the other hand, the advantage of facet is that it "looks" almost like face:). So if you only need to work with facets, I don't think that will be too confusing if it is mentioned clearly that p4est use face for what we call facet...

I'm talking about naming things that already have an "appropriate" name in the paper. For example algorithm 5,6 or algorithm 8,9,10 (https://p4est.github.io/papers/BursteddeWilcoxGhattas11.pdf). IMHO it would be another layer of complexity that would prevent other people from getting into that part of the code and contributing, since you'd not only have to map the paper algorithms to the implementation, but also have to worry about mapping the renaming. There is already a Ferrite->p4est and p4est->Ferrite mapping in the codebase for entity indices that are numbered differently. Having another permutation seems a bit complicated to me. Maybe that's just my bias.

@koehlerson
Copy link
Member Author

talked with Dennis about it, we will rename the face stuff to facet and document that change

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

5 participants