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

docs contribution #1098

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/L2_projection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,13 @@ function assemble_proj_rhs!(f::Matrix, cellvalues::CellValues, sdh::SubDofHandle
end
end

"""
evaluate_at_grid_nodes(proj::L2Projector, vals::AbstractVector)
Return a vector of length `getnnodes(grid)` where the order corresponds with the node order
in the `grid` used to create `proj`.

`vals` must have length `getnnodes(grid)`.
Copy link
Member

Choose a reason for hiding this comment

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

The length can be different from getnnodes.

Suggested change
`vals` must have length `getnnodes(grid)`.
`vals` should be the output from `project` using `proj`.

"""
evaluate_at_grid_nodes(proj::L2Projector, vals::AbstractVector) =
_evaluate_at_grid_nodes(proj, vals, Val(false))

Expand Down
4 changes: 2 additions & 2 deletions src/Quadrature/quadrature.jl
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ using Base.Cartesian: @nloops, @ntuple, @nexprs
QuadratureRule{shape}(weights::AbstractVector{T}, points::AbstractVector{Vec{rdim, T}})

Create a `QuadratureRule` used for integration on the refshape `shape` (of type [`AbstractRefShape`](@ref)).
`order` is the order of the quadrature rule.
`quad_rule_type` is an optional argument determining the type of quadrature rule,
`order` is related to the number of quadrature points per direction (i.e. a `QuadratureRule{RefQuadrilateral}(2)`
uses 2 quadrature points per direction). `quad_rule_type` is an optional argument determining the type of quadrature rule,
Comment on lines +18 to +19
Copy link
Member

Choose a reason for hiding this comment

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

This is only true for hypercubes. For simplexes the concept of "each direction" is a bit harder to describe. I don't think we are fully consistent with what is meant with order, see also #1022 for some discussions about the options. For now, would something like this explain good enough?

Suggested change
`order` is related to the number of quadrature points per direction (i.e. a `QuadratureRule{RefQuadrilateral}(2)`
uses 2 quadrature points per direction). `quad_rule_type` is an optional argument determining the type of quadrature rule,
`order` is related to the number of quadrature points, but is dependent on the reference shape.
`quad_rule_type` is an optional argument determining the type of quadrature rule,

Copy link
Collaborator

@DRollin DRollin Nov 6, 2024

Choose a reason for hiding this comment

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

I discussed that with Phil: We thought the general "is related to" would be enough to make clear that this is not always exactly true
Now, I see that this is still not perfectly clear.
What you suggest could be read as "The order depends on the reference shape." But this is not what you mean, right?

Copy link
Member

Choose a reason for hiding this comment

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

Ohh, good catch here! Not sure how we missed that inconsistency earlier. We should definitely clarify this.

When writing the docs I found it difficult to make a balance between 1) being precise and 2) being still useful for users which are not fully aware (and interested) of all details. So for everything but hypercubes we refer with order to the actual integration order of the rule w.r.t. to the function system it integrates. Note that the function system varies between rules, see for example the Gauss-Laguerre rule for an example of a fancy system. So, if you have any idea how to communicate what "order" means to users, I am open for any suggestion.

Copy link
Member

@KnutAM KnutAM Nov 6, 2024

Choose a reason for hiding this comment

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

Good point!
Something like this, and then we should document the rules somewhere and point to that location in the docstring. Probably nice not having all rules documented in the docstring as this would get too verbose.

Suggested change
`order` is related to the number of quadrature points per direction (i.e. a `QuadratureRule{RefQuadrilateral}(2)`
uses 2 quadrature points per direction). `quad_rule_type` is an optional argument determining the type of quadrature rule,
`order` affects the number of quadrature points, and thus the precision of the integration rule. The exact relationship between `order` and the number of quadrature points depends on the `quad_rule_type` and `shape`.
`quad_rule_type` is an optional argument determining the type of quadrature rule,

Copy link
Collaborator

@DRollin DRollin Nov 7, 2024

Choose a reason for hiding this comment

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

I think this is nice for the docstring 👍
Maybe we could add a more detailed description (hypercubes vs. other shapes) somewhere in the documentation?
Suggestions where to add it:

  1. at the beginning of "Topic guides/Reference shapes"

  2. at the end of "Topic guides/Reference shapes"

  3. in the tutorial "Heat equation"

currently the `:legendre` and `:lobatto` rules are implemented for hypercubes.
For triangles up to order 8 the default rule is the one by `:dunavant` (see [Dun:1985:hde](@cite)) and for
tetrahedra the default rule is `keast_minimal` (see [Keast:1986:mtq](@cite)). Wedges and pyramids default
Expand Down
Loading