docs contribution#1098
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1098 +/- ##
=======================================
Coverage 94.40% 94.40%
=======================================
Files 40 40
Lines 6846 6846
=======================================
Hits 6463 6463
Misses 383 383 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Welcome @kreutzheidephil 🚀
Some comments below. It will also be required to merge with the master branch.
After that, need to fix formatting to remove errors reported by the "pre-commit" workflow (and later "runic" after merge with master)
| `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, |
There was a problem hiding this comment.
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?
| `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, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| `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, |
There was a problem hiding this comment.
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:
| 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)`. |
There was a problem hiding this comment.
The length can be different from getnnodes.
| `vals` must have length `getnnodes(grid)`. | |
| `vals` should be the output from `project` using `proj`. |
Uh oh!
There was an error while loading. Please reload this page.