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

Fix show of Face and Cell values #771

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Fix show of Face and Cell values #771

merged 3 commits into from
Jul 25, 2023

Conversation

KnutAM
Copy link
Member

@KnutAM KnutAM commented Jul 24, 2023

For upcoming 1.0, make sure show doesn't fail for face values (closes #752)
Makes output for CellValues consistent with FaceValues.

Note: Exact output may change later (e.g. if/when we split #764 or similar), this is just to remove bug for the upcoming release.

Output:

# Scalar
CellValues with
- Quadrature rule with 4 points
- Function interpolation: Lagrange{RefQuadrilateral, 2}()
- Geometric interpolation: Lagrange{RefQuadrilateral, 1}()

# Vector 
CellValues with
- Quadrature rule with 4 points
- Function interpolation: Lagrange{RefQuadrilateral, 2}()^2
- Geometric interpolation: Lagrange{RefQuadrilateral, 1}()

FaceValues with
- Quadrature rule with 2 points per face
- Function interpolation: Lagrange{RefQuadrilateral, 2}()
- Geometric interpolation: Lagrange{RefQuadrilateral, 1}()

# Different number of quadrature points on each face:
FaceValues with
- Quadrature rule with (3, 2, 2, 2) points on each face
- Function interpolation: Lagrange{RefQuadrilateral, 2}()
- Geometric interpolation: Lagrange{RefQuadrilateral, 1}()

Copy link
Member

@termi-official termi-official left a comment

Choose a reason for hiding this comment

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

Maybe the smoke test should be with a prism or pyramid to capture possible regression here. Otherwise really nice fix!

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.49% 🎉

Comparison is base (a014185) 91.77% compared to head (7cb52a0) 92.27%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #771      +/-   ##
==========================================
+ Coverage   91.77%   92.27%   +0.49%     
==========================================
  Files          32       32              
  Lines        4720     4735      +15     
==========================================
+ Hits         4332     4369      +37     
+ Misses        388      366      -22     
Files Changed Coverage Δ
src/FEValues/cell_values.jl 100.00% <100.00%> (ø)
src/FEValues/face_values.jl 98.27% <100.00%> (+0.14%) ⬆️

... and 2 files with indirect coverage changes

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

@KnutAM
Copy link
Member Author

KnutAM commented Jul 25, 2023

Maybe the smoke test should be with a prism or pyramid to capture possible regression here

Added for CellValues only since ERROR: FaceQuadratureRule for RefPrism not implemented (#772)

@KnutAM KnutAM merged commit debf386 into master Jul 25, 2023
@KnutAM KnutAM deleted the kam/FEValues_show branch July 25, 2023 06:19
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.

Showing FaceValues in the REPL is broken
3 participants