MOLFiniteDifference : Adding support for higher derivative orders in bcs #435
Conversation
|
For use cases like #344 , #385 etc. support for N-ordered derivatives has been provided. |
There was a problem hiding this comment.
Can you add a test with Neumann BCs to this? https://github.com/SciML/DiffEqOperators.jl/blob/master/test/MOL/MOL_2D_Diffusion.jl
| # TODO: Fix Neumann and Robin on higher dimension | ||
| lhs = nspace == 1 ? substitute(bc.lhs,depvarderivbcmaps[i]) : bc.lhs | ||
| # Update: Fixed for 1D systems |
There was a problem hiding this comment.
can remove these two comments imo
There was a problem hiding this comment.
Can you add a test with Neumann BCs to this? https://github.com/SciML/DiffEqOperators.jl/blob/master/test/MOL/MOL_2D_Diffusion.jl
Yeah, so the support is for higher ordered Neumann in 1D systems.
Perhaps a test should be added in :
https://github.com/SciML/DiffEqOperators.jl/blob/master/test/MOL/MOL_1D_HigherOrder.jl ?
There was a problem hiding this comment.
can remove these two comments imo
I think that is still a TODO
|
@tinosulzer tests added. |
There was a problem hiding this comment.
Sorry I misunderstood what you were doing. Yes, the tests should be in https://github.com/SciML/DiffEqOperators.jl/blob/master/test/MOL/MOL_1D_HigherOrder.jl (ideally just fix the tests that are test_broken )
test/MOL/MOL_1D_Linear_Diffusion.jl
Outdated
| bcs = [u(0,x) ~ cos(x), | ||
| Dxx(u(t,0)) ~ -exp(-t), | ||
| Dxx(u(t,Float64(π))) ~ exp(-t)] |
There was a problem hiding this comment.
You can't specify a 2nd order BC on a 2nd order system. See #382 (comment)
There was a problem hiding this comment.
Yes, this was just to show that desired operation gets carried out safely and in correct manner.
But yeah, I think a higher ordered eqn would be good.
test/MOL/MOL_1D_Linear_Diffusion.jl
Outdated
| bcs = [u(0,x) ~ cos(x), | ||
| v(0,x) ~ sin(x), | ||
| u(t,0) ~ exp(-t), | ||
| Dxxx(u(t,1)) ~ exp(-t) * sin(1), |
There was a problem hiding this comment.
BC order should be <= 1
Ideally I should have fixed those broken tests but I think some other pieces are missing in handling beam eqns (like those mentioned in #382 (comment) etc.) 😅 |
|
I think the KdV wave eqn should be a good test case. The no. of BCs have been set more than required though, to produce an accurate solution. |
|
@tinosulzer this looks good? |
test/MOL/MOL_1D_HigherOrder.jl
Outdated
| u_real = [[u_analytic(x, t) for x in xs] for t in ts] | ||
| u_diff = u_real - u_predict | ||
| for i in 1:length(u_diff) | ||
| @test u_diff[i] ≈ zeros(length(u_diff[i])) atol=0.05; |
There was a problem hiding this comment.
Looks good, can the other (broken) KS test now be removed? And why do the beam equation tests still not work after this fix?
Yes, we can actually remove the KS test. I tested that, it would just require more BCs to pass but that seems unnecessary since we already have the KdV test.
As for the beam eqn, thats related to #382 (comment) point 2.
No description provided.