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

Update some missing Lagrange docs #889

Merged
merged 15 commits into from
Jul 12, 2024
Merged

Conversation

termi-official
Copy link
Member

No description provided.

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.60%. Comparing base (baf8aad) to head (4437a85).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #889   +/-   ##
=======================================
  Coverage   93.60%   93.60%           
=======================================
  Files          39       39           
  Lines        5879     5883    +4     
=======================================
+ Hits         5503     5507    +4     
  Misses        376      376           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Nice!
Just some formatting fix suggestions while we are at it...

docs/src/topics/degrees_of_freedom.md Outdated Show resolved Hide resolved
docs/src/topics/degrees_of_freedom.md Outdated Show resolved Hide resolved
docs/src/topics/grid.md Outdated Show resolved Hide resolved
docs/src/topics/grid.md Outdated Show resolved Hide resolved
Co-authored-by: Knut Andreas Meyer <knutam@gmail.com>
@termi-official termi-official marked this pull request as draft March 4, 2024 09:39
@termi-official termi-official requested a review from KnutAM June 3, 2024 14:48
@termi-official termi-official marked this pull request as ready for review June 3, 2024 14:48
@termi-official
Copy link
Member Author

I noticed that on master some things can be now done in a better way (i.e. without relying on assumptions taken between the interpolation and cell) so I updated it. Also some dispatches were missing, which I added for completeness of the API.

@termi-official
Copy link
Member Author

I just saw that #935 has quite a bit of overlap here, so we need to decide on a merge order.

@termi-official
Copy link
Member Author

CI failure seems to be the github CDN or CI servers choking

ERROR: LoadError: LoadError: LoadError: Operation too slow. Less than 1 bytes/sec transferred the last 20 seconds while requesting https://raw.githubusercontent.com/Ferrite-FEM/Ferrite.jl/gh-pages/assets/transient_heat.gif

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.

LGTM if if removing changes in grid.md as discussed on Slack, and fixes the comments

src/Grid/grid.jl Outdated Show resolved Hide resolved
docs/src/devdocs/elements.md Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
docs/src/devdocs/elements.md Outdated Show resolved Hide resolved
docs/src/devdocs/elements.md Outdated Show resolved Hide resolved
src/Grid/grid.jl Show resolved Hide resolved
src/Grid/grid.jl Show resolved Hide resolved
@termi-official termi-official added the awaiting review PR is finished from the authors POV, waiting for feedback label Jul 9, 2024
@termi-official
Copy link
Member Author

Example to be reinserted separately.

Now to a full example. Let us reconstruct the local facets of a grid boundary with the described machinery to see how everything
fits together:

julia> function recompute_facetset_on_boundary(cells, global_facets)
           # Facets in 2D are edges and have two vertices
           d = Dict{NTuple{2, Int}, FacetIndex}()
           for (c, cell) in enumerate(cells) # c is global cell number
               # Grab all local facets
               local_facets = Ferrite.reference_facets(cell)
               for (f, facet) in enumerate(local_facets) # f is local facet number
                   # Translate into global nodes
                   global_facet = ntuple(i-> cell.nodes[facet[i]], length(facet))
                   d[global_facet] = FacetIndex(c, f)
               end
           end

           facets = Vector{FacetIndex}()
           for facet in global_facets
               # lookup the element, local facet combination for this facet
               push!(facets, d[facet])
           end

           return facets
       end

julia> recompute_facetset_on_boundary(cells, facets)
Vector{FacetIndex} with 2 elements:
  FacetIndex((2, 2))
  FacetIndex((4, 2))

Note that this example can be simplified by directly using Ferrite.facets:

julia> function recompute_facetset_on_boundary_alternative(cells, global_facets)
           # Facets in 2D are edges and have two vertices
           d = Dict{NTuple{2, Int}, FacetIndex}()
           for (c, cell) in enumerate(cells) # c is global cell number
               # Grab all local facets
               for (f, global_facet) in enumerate(Ferrite.facets(cell)) # f is local facet number
                   d[global_facet] = FacetIndex(c, f)
               end
           end

           facets = Vector{FacetIndex}()
           for facet in global_facets
               # lookup the element, local facet combination for this facet
               push!(facets, d[facet])
           end

           return facets
       end

julia> recompute_facetset_on_boundary_alternative(cells, facets)
Vector{FacetIndex} with 2 elements:
  FacetIndex((2, 2))
  FacetIndex((4, 2))

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.

Spotted a few more things when going through carefully

docs/src/devdocs/reference_cells.md Outdated Show resolved Hide resolved
docs/src/topics/grid.md Outdated Show resolved Hide resolved
docs/src/topics/grid.md Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
docs/src/devdocs/reference_cells.md Outdated Show resolved Hide resolved
docs/src/devdocs/reference_cells.md Outdated Show resolved Hide resolved
docs/src/devdocs/reference_cells.md Outdated Show resolved Hide resolved
termi-official and others added 5 commits July 10, 2024 12:24
Co-authored-by: Knut Andreas Meyer <knutam@gmail.com>
Co-authored-by: Knut Andreas Meyer <knutam@gmail.com>
…le) info. A future PR should introduce a page with more context and add the reference to this page into the docstring.
…le) info. A future PR should introduce a page with more context and add the reference to this page into the docstring.
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.

Nice!

A lot of discussion for a small docs PR :)
But I think we ended up with some quite important improvements / clarifications in the end!

src/Grid/grid.jl Outdated Show resolved Hide resolved
src/Grid/grid.jl Outdated Show resolved Hide resolved
@KnutAM KnutAM merged commit e31e220 into master Jul 12, 2024
11 checks passed
@KnutAM KnutAM deleted the do/update-missing-some-lagrange-docs branch July 12, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review PR is finished from the authors POV, waiting for feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants