-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add AnalysisSurfaceIntegral #1812
Add AnalysisSurfaceIntegral #1812
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1812 +/- ##
==========================================
+ Coverage 96.30% 96.32% +0.02%
==========================================
Files 441 445 +4
Lines 35799 35926 +127
==========================================
+ Hits 34476 34605 +129
+ Misses 1323 1321 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for bringing this functionality to Trixi!
I added some first mostly implementation-related comments.
Please also add a test for the new elixir, such that the newly added code becomes part of our CI runs.
2badb17
to
9e95c37
Compare
1a704d6
to
78f22d1
Compare
0621431
to
ac87bed
Compare
See #1842. |
Ah, thank you so much! |
004fa97
to
bc04f60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a comment to open a discussion on how to grab the boundary indices that should be integrated over.
3276bbb
to
9b0915a
Compare
Co-authored-by: Daniel Doehring <[email protected]>
@Arpit-Babbar I took the liberty to push this a little further as I want to do some actual aerodynamic simulations which also involve lift (and especially drag) coefficients due to viscous forces. For this, the coefficients due to pressure should be merged first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very nice additional feature. Most of my comments / suggestions are related to notation and variable names.
examples/p4est_2d_dgsem/elixir_euler_NACA0012airfoil_mach085.jl
Outdated
Show resolved
Hide resolved
examples/p4est_2d_dgsem/elixir_euler_NACA0012airfoil_mach085.jl
Outdated
Show resolved
Hide resolved
examples/p4est_2d_dgsem/elixir_euler_NACA0012airfoil_mach085.jl
Outdated
Show resolved
Hide resolved
examples/p4est_2d_dgsem/elixir_euler_NACA0012airfoil_mach085.jl
Outdated
Show resolved
Hide resolved
Some key points 1) Change pre to p 2) Don't capitalize U 3) Don't append Swanson quantities with 'sw' 4) linf -> l_inf (may be pending in some other places) 5) Other formatting improvements Co-authored-by: Andrew Winters <[email protected]>
examples/p4est_2d_dgsem/elixir_euler_NACA0012airfoil_mach085.jl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Many thanks @Arpit-Babbar och @DanielDoehring this is a very nice feature to add to Trixi.
This PR adds a callback to compute forces on the surface of a body, like the surface of an airfoil. Three examples, each having steady state solutions, have been added