Adding primitive variable Dirichlet BCs for Navier-Stokes parabolic terms for P4estMesh{2}
#1553
Conversation
jlchan
changed the title
P4estMesh{2}P4estMesh{2}
|
@apey236 the formatting check failed. Can you use JuliaFormatter.jl to format the changed files? You should also be able to format the |
Codecov Report
@@ Coverage Diff @@
## main #1553 +/- ##
==========================================
+ Coverage 92.90% 96.14% +3.24%
==========================================
Files 402 403 +1
Lines 33034 33158 +124
==========================================
+ Hits 30690 31879 +1189
+ Misses 2344 1279 -1065
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Jesse Chan <[email protected]>
Co-authored-by: Jesse Chan <[email protected]>
|
@apey236 when you are ready, please re-request a review from me. @ranocha @andrewwinters5000 if either of you have time, it would be great to get a review from you on this as well. |
There was a problem hiding this comment.
Seems reasonable but a review from @andrewwinters5000 would be nice
|
@andrewwinters5000 Do you have time for a review? |
|
Yes, will look at it tomorrow |
|
Interesting; all functions causing CI issues were tagged with Edit: #1593 |
There was a problem hiding this comment.
LGTM, I assume that you have run convergence_test and verified that you get the correct EOCs. It will be useful to have these boundary conditions and a convergence elixir available on P4estMesh for testing the further development of the parabolic mortars implementation from #1571
|
I don't think we checked |
|
Bump - @apey236 can you post the results of |
|
@andrewwinters5000 > LGTM, I assume that you have run Here is the convergence test on "/p4est_2d_dgsem/elixir_navierstokes_convergence_nonperiodic.jl" example: #################################################################################################### mean 5.70 mean 6.56 mean 6.66 mean 5.13linf mean 5.20 mean 6.35 mean 5.46 mean 4.03Dict{Symbol, Any} with 3 entries: |
There was a problem hiding this comment.
Interesting, the EOCs look fine from the convergence test but perhaps a bit better than expected. For a polydeg=3 we typically see that a mesh refinement study, like the one you have performed, gives an EOC of approximately p+1 (so 4 in this case). That you are achieving an EOC of p+2 is an unexpected surprise.
I think it is because the manufactured solution rho = c + A * sin(pi_x) * cos(pi_y) * cos(pi_t) is approximately 0.0 at the final time you chose in the tspan=(0.0, 0.5). It is always dangerous to test convergence around an exact solution near zero because of round-off errors. I am relatively sure that the implementation of the new BCs is working properly. However, and I know it is a pain, could you rerun the EOC test with tspan=(0.0, 0.75)? For this I expect that the EOC values will be closer to the expected p+1 values.
@andrewwinters5000, you are right. I changed the time span to 0.75 and now here is the results of the convergence test: #################################################################################################### mean 3.90 mean 4.33 mean 4.32 mean 3.83linf mean 3.71 mean 4.26 mean 4.09 mean 3.87Dict{Symbol, Any} with 3 entries: |
jlchan
dismissed stale reviews from andrewwinters5000 and themself
via
ab9da82
|
@ranocha @andrewwinters5000 we previously approved this PR, but I forgot to merge. After updating, it looks like the MPI and threaded windows tests are failing, but the issues don't seem related to this PR. OK to merge or would you like us to wait until CI is fixed? |
|
We can merge this now. The same kind of CI failures appear in other PRs |
No description provided.