-
-
Notifications
You must be signed in to change notification settings - Fork 74
[WIP] Correct concretization BCs and ghost derivative operators #185
base: master
Are you sure you want to change the base?
Conversation
26dee6f
to
e587bb0
Compare
|
||
Base.convert(::Type{Array},A::AbstractBC) = Array(A) | ||
Base.convert(::Type{SparseMatrixCSC},A::AbstractBC) = SparseMatrixCSC(A) | ||
Base.convert(::Type{AbstractMatrix},A::AbstractBC) = SparseMatrixCSC(A) |
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.
IMO, this should act like sparse
and give "the best choice"
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.
Well, none of these convert
methods actually work at all, since the matrix constructors require a length argument N
to concretize a boundary condition, and there is no possible default length.
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.
Then maybe it's best to just delete them?
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.
Will do.
|
||
Base.convert(::Type{Array},A::AbstractBC) = Array(A) | ||
Base.convert(::Type{SparseMatrixCSC},A::AbstractBC) = SparseMatrixCSC(A) | ||
Base.convert(::Type{AbstractMatrix},A::AbstractBC) = SparseMatrixCSC(A) |
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.
Base.convert(::Type{AbstractMatrix},A::AbstractBC) = SparseMatrixCSC(A) | |
Base.convert(::Type{AbstractMatrix},A::AbstractBC) = sparse(A) |
Just a heads up, The |
@xtalax Thanks for the heads-up! As you see in the latest commit to this PR, I did not do anything big, just cleaned the concretization code. However, I did not yet figure out how to shrink the bandwidths properly. You can remove julia> L = 10.0
10.0
julia> N = 9
9
julia> δx = L/(N+1)
1.0
julia> Δ = CenteredDifference(2, 2, δx, N)
DerivativeOperator{Float64,1,false,Float64,StaticArrays.SArray{Tuple{3},Float64,1,3},StaticArrays.SArray{Tuple{0},StaticArrays.SArray{Tuple{4},Float64,1,4},1,0},Nothing,Nothing}(2, 2, 1.0, 9, 3, [1.0, -2.0, 1.0], 4, 0, StaticArrays.SArray{Tuple{4},Float64,1,4}[], StaticArrays.SArray{Tuple{4},Float64,1,4}[], nothing, nothing)
julia> Q = Dirichlet0BC(typeof(δx))
RobinBC{Float64,Array{Float64,1}}([-0.0, 0.0], 0.0, [-0.0, 0.0], 0.0)
julia> A = Δ*Q;
julia> first(sparse(A))
9×9 BandedMatrix{Float64,Array{Float64,2},Base.OneTo{Int64}}:
-2.0 1.0 ⋅ ⋅ ⋅ ⋅ ⋅ ⋅ ⋅
1.0 -2.0 1.0 ⋅ ⋅ ⋅ ⋅ ⋅ ⋅
⋅ 1.0 -2.0 1.0 ⋅ ⋅ ⋅ ⋅ ⋅
⋅ ⋅ 1.0 -2.0 1.0 ⋅ ⋅ ⋅ ⋅
⋅ ⋅ ⋅ 1.0 -2.0 1.0 ⋅ ⋅ ⋅
⋅ ⋅ ⋅ ⋅ 1.0 -2.0 1.0 ⋅ ⋅
⋅ ⋅ ⋅ ⋅ ⋅ 1.0 -2.0 1.0 ⋅
⋅ ⋅ ⋅ ⋅ ⋅ ⋅ 1.0 -2.0 1.0
⋅ ⋅ ⋅ ⋅ ⋅ ⋅ ⋅ 1.0 -2.0 However, Neumann BCs (et al.) do not generate optimal bandwidths, since banded–banded multiplication has no way of knowing that most of the sub-/superpdiagonals are actually zero: julia> Q = Neumann0BC(δx)
RobinBC{Float64,Array{Float64,1}}([1.0], -0.0, [1.0], 0.0)
julia> A = Δ*Q;
julia> first(sparse(A))
9×9 BandedMatrix{Float64,Array{Float64,2},Base.OneTo{Int64}}:
-1.0 1.0 0.0 ⋅ ⋅ ⋅ ⋅ ⋅ ⋅
1.0 -2.0 1.0 0.0 ⋅ ⋅ ⋅ ⋅ ⋅
0.0 1.0 -2.0 1.0 0.0 ⋅ ⋅ ⋅ ⋅
⋅ 0.0 1.0 -2.0 1.0 0.0 ⋅ ⋅ ⋅
⋅ ⋅ 0.0 1.0 -2.0 1.0 0.0 ⋅ ⋅
⋅ ⋅ ⋅ 0.0 1.0 -2.0 1.0 0.0 ⋅
⋅ ⋅ ⋅ ⋅ 0.0 1.0 -2.0 1.0 0.0
⋅ ⋅ ⋅ ⋅ ⋅ 0.0 1.0 -2.0 1.0
⋅ ⋅ ⋅ ⋅ ⋅ ⋅ 0.0 1.0 -1.0 No tests for this yet. |
# possible, and we accomplish this by dropping all trailing | ||
# zeros. This way, we do not write outside the bands of the | ||
# BandedMatrix. | ||
a_r = Q.a_r[1:something(findlast(!iszero, Q.a_r), 0)] |
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.
Why isn't this something(findfirst(!iszero, Q.a_r):end
?
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.
Don't know :) I have not thought about this PR since September.
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 just noticed hat I had started a review and not submitted it, probably also since September - better late than never :P
Fixes #182.
So far, the different concretizations of boundary conditions have been implemented, ghost derivative operators are up next.
Please check
u
forPeriodicBC
s, I was unsure if they should be padded withzero(T)
orone(T)
; they were of incorrect lengthN
, need to beN+2
.