-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Hybrid CG DG #23986
Hybrid CG DG #23986
Conversation
@lindsayad Thanks, I’ll see how it works with what I was testing tomorrow. I agree it looks promising, especially for my two-phase app where I need to make sure face values are bounded by the cell-centered values when interfacial drag dominates the eqs. |
The preliminary work I did here was only for pressure and velocity and I haven't even added any coefficients yet. So if you think you might want to use/extend what is here, let me know |
@joe61vette in c74e815 I added a hybrid scheme test for the same skewed mesh that we test our finite volume skew correction methodology on. The hybrid scheme maintains the same orders of accuracy for skewed meshes as for non-skewed meshes, e.g.
|
8658f37
to
8cbfabb
Compare
Job Documentation on 360f0d3 wanted to post the following: View the site here This comment will be updated on new commits. |
Job Coverage on 360f0d3 wanted to post the following: Framework coverage
Modules coverageNavier stokes
Full coverage reportsReports
This comment will be updated on new commits. |
fcc37f2
to
4d02831
Compare
62c7c8d
to
b56c8a1
Compare
5825c80
to
593bf42
Compare
593bf42
to
5e6bd52
Compare
Job Precheck on 5e6bd52 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
fe15732
to
c6f1ef9
Compare
Job Conda build (ARM Mac) on 0e53445 : invalidated by @lindsayad |
Job Framework 1 on 0e53445 : invalidated by @lindsayad |
ff74c37
to
225f949
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'm pretty thrilled with the code, and astonished that I'd never seen someone using this formulation before ... but could we get some navier_stokes/doc/content/source/kernels/ entries for the new kernels?
Pretty much all the kernels added in this PR are test objects. The objects used for "production" are very general from the framework. I will make a markdown file though that runs through one of the "production" inputs in detail |
2abc34e
to
32f6d7d
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.
hcgdgfe.md looks good enough for me. Why'd you have to force push, though?
modules/navier_stokes/doc/content/modules/navier_stokes/hcgdgfe.md
Outdated
Show resolved
Hide resolved
modules/navier_stokes/doc/content/modules/navier_stokes/hcgdgfe.md
Outdated
Show resolved
Hide resolved
modules/navier_stokes/doc/content/modules/navier_stokes/hcgdgfe.md
Outdated
Show resolved
Hide resolved
modules/navier_stokes/doc/content/modules/navier_stokes/hcgdgfe.md
Outdated
Show resolved
Hide resolved
modules/navier_stokes/doc/content/modules/navier_stokes/hcgdgfe.md
Outdated
Show resolved
Hide resolved
modules/navier_stokes/test/tests/finite_element/ins/cg-dg-hybrid/mms/lid-driven-skewed/tests
Outdated
Show resolved
Hide resolved
I rebased as I was trying to figure out what turned out to be unrelated local failures |
- Fix typos - Add links to more documentation pages Co-authored-by: Guillaume Giudicelli <guillaume.giudicelli@gmail.com>
30531a1
to
360f0d3
Compare
Job Conda (Intel Mac) on 360f0d3 : invalidated by @lindsayad |
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'm happy but give Guillaume a chance to chime in before merging.
I will probably merge this by the end of the day if I don't hear anything |
Apologies. I will review this again tomorrow. Maybe tonight. |
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.
looks good
if this approach keeps being successful (robustness especially), it makes a ton of sense to do "coarse mesh" thermal hydraulics with higher order FE (instead of flat FV requiring us to resolve quite a bit) so it could be another Pgh change of direction :)
Thanks for the reviews. I did test this with a porosity jump and similar to a naive finite volume implementation, it struggles, so no easy silver bullet for that problem :-) |
From pull request idaholab#23986. Refs issue idaholab#24055
@joe61vette I was so intrigued by what you're working on that I had to test it myself.
If we ever want to go with a scheme that doesn’t require Rhie-Chow nor petrov-galerkin stabilization, and allows discontinuous boundary conditions on velocity, the hybrid CG-DG approach appears pretty damn cool. I spun up some kernels today and see second and first order L2 errors for velocity and pressure respectively when using first monomial and first Lagrange for velocity and pressure respectively. And third and second order L2 errors for velocity and pressure if I bump the velocity order to SECOND (keeping pressure at FIRST).
Lid driven result for Re=100 (just using averaging of face values for the advection scheme):
Closes #24055