Implement periodic boundary conditions #83
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #83 +/- ##
==========================================
+ Coverage 89.47% 90.23% +0.75%
==========================================
Files 9 9
Lines 399 430 +31
==========================================
+ Hits 357 388 +31
Misses 42 42 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looks good overall.
- There are quite a few unused functions and features. Let's cut those out and add them as a separate PR if needed.
- I'm not sure why, but codecov doesn't think these tests are working. The coverage is only showing at 65% of the new lines, which is way too low.
src/Body.jl
Outdated
| @@ -49,9 +49,10 @@ function measure!(a::Flow{N,T},body::AbstractBody;t=T(0),ϵ=1) where {N,T} | |||
| end | |||
| end | |||
| @loop fill!(a.μ₀,a.μ₁,a.V,a.σᵥ,a.σ,I) over I ∈ inside(a.p) | |||
| BC!(a.μ₀,zeros(SVector{N,T}),a.exitBC,a.perdir;Dirichlet=false) # add BC care for periodic & neu for μ₀ | |||
There was a problem hiding this comment.
I don't understand these comments...
src/Flow.jl
Outdated
| @@ -4,6 +4,7 @@ | |||
| @fastmath quick(u,c,d) = median((5c+2d-u)/6,c,median(10c-9u,c,d)) | |||
| @fastmath vanLeer(u,c,d) = (c≤min(u,d) || c≥max(u,d)) ? c : c+(d-c)*(c-u)/(d-u) | |||
| @inline ϕu(a,I,f,u,λ=quick) = @inbounds u>0 ? u*λ(f[I-2δ(a,I)],f[I-δ(a,I)],f[I]) : u*λ(f[I+δ(a,I)],f[I],f[I-δ(a,I)]) | |||
| @inline ϕuSelf(I1,I2,I3,I4,f,u,λ=quick) = @inbounds u>0 ? u*λ(f[I1],f[I2],f[I3]) : u*λ(f[I4],f[I3],f[I2]) | |||
There was a problem hiding this comment.
This can actually be written in a neater way, there is actually only I-2δ(a,I) that needs a special treatment.
| @@ -110,14 +128,14 @@ and the `AbstractPoisson` pressure solver to project the velocity onto an incomp | |||
| @fastmath function mom_step!(a::Flow,b::AbstractPoisson) | |||
| a.u⁰ .= a.u; scale_u!(a,0) | |||
| # predictor u → u' | |||
| conv_diff!(a.f,a.u⁰,a.σ,ν=a.ν) | |||
| BDIM!(a); BC!(a.u,a.U,a.exitBC) | |||
| conv_diff!(a.f,a.u⁰,a.σ,ν=a.ν,perdir=a.perdir) | |||
There was a problem hiding this comment.
These function arguments are getting pretty long... It's fine, I guess
There was a problem hiding this comment.
That's true, this will be resolved by @generated functions further down the line I suppose.
src/util.jl
Outdated
| end | ||
|
|
||
| """ | ||
| interp(x::SVector, arr::AbstractArray) |
There was a problem hiding this comment.
Why is this function part of the PR?
| @allowscalar @test all(u[end, :, 1] .== 3) | ||
|
|
||
| WaterLily.exitBC!(u,u,U,0) # conservative exit check | ||
| @allowscalar @test all(u[end,2:end-1, 1] .== U[1]) | ||
|
|
||
| BC!(u,U,true,(2,)) # periodic in y and save exit values |
There was a problem hiding this comment.
Not sure why, but the code cov tool doesn't seem to think these tests are actually running the new code. Give it a look.
There was a problem hiding this comment.
Yes, I saw that the codecov doesn't register a few lines
test/runtests.jl
Outdated
| @@ -98,20 +110,20 @@ end | |||
| @test_throws AssertionError("MultiLevelPoisson requires size=a2ⁿ, where n>2") Poisson_setup(MultiLevelPoisson,(15+2,3^4+2)) | |||
|
|
|||
| err,pois = Poisson_setup(MultiLevelPoisson,(10,10)) | |||
| @test pois.levels[3].D == Float32[0 0 0 0; 0 -2 -2 0; 0 -2 -2 0; 0 0 0 0] | |||
| @test pois.levels[3].D == Float32[0 0 0 0; 0 -2 -3 0; 0 -3 -4 0; 0 0 0 0] | |||
There was a problem hiding this comment.
This is actually a bad error, I didn't realise when I did that, but it's related the the BC on the \mu_0 field in the levels below 1 of the multigrid.
|
These two commits should do the trick, I think I have addressed all the issues and I tried to improve the coverage of the tests. |
Implementation of periodic boundary conditions together with the convective exit.
There is an example examples/TwoD_circle_periodicBC_convectiveBC.jl that runs a y-periodic flow with a convective exit. The circle moves in the y-direction and crosses the boundary. This shows how to set up a periodic sdf.
I have added tests to check that the correct behavior is obtained for the boundary condition, for the different combinations of input.
I have not tested that the QUICK scheme implementation for periodic BC is correct, I just assumed it is based on @TzuYaoHuang simulations.
One caveat: there is no check that x-periodic flow and convective exit are not used at the same time, but because this will default to periodic, it should be fine.