-
-
Notifications
You must be signed in to change notification settings - Fork 72
[WIP] First steps in PDE autodiscretization. #250
[WIP] First steps in PDE autodiscretization. #250
Conversation
…rc/MOL_discretization.jl). 1D toy examples added (test/): linear convection and diffusion.
|
Looking great. @YingboMa comments? |
From ChrisRackauckas: you don't need to allocate the array here. This will use a lazy concatenation to decrease the memory cost. Co-authored-by: Christopher Rackauckas <accounts@chrisrackauckas.com>
…tion.jl) to runtests.jl. Minor changes in these tests.
|
I'll look into the unrelated test failures on master. |
|
Retriggering build due to upstream fixes |
YingboMa
left a comment
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 commented on a few stylistic things. I don't quite understand what extract_bc is doing.
|
Rebase since master now passes tests |
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
Co-authored-by: Yingbo Ma <mayingbo5@gmail.com>
…uellujan/DiffEqOperators.jl into wip_pde_autodiscretization
src/MOL_discretization.jl
Outdated
| function discretize_2(input,grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| if input isa ModelingToolkit.Constant | ||
| return :($input.value) | ||
| elseif input isa Operation | ||
| if input.op isa Variable | ||
| if haskey(lhs_nonderiv_depvars,input.op) | ||
| x = lhs_nonderiv_depvars[input.op] | ||
| if x isa ModelingToolkit.Constant | ||
| expr = :($x.value) | ||
| else | ||
| expr = Expr(x) | ||
| expr = :(x=i*$dx;eval($expr)) | ||
| end | ||
| elseif grade == 1 | ||
| # TODO: the discretization order should not be the same for | ||
| # first derivatives and second derivarives | ||
| j = findfirst(x->x==input.op, lhs_deriv_depvars) | ||
| expr = :((u[$j][i]-u[$j][i-1])/$dx) | ||
| elseif grade == 2 | ||
| j = findfirst(x->x==input.op, lhs_deriv_depvars) | ||
| expr = :((u[$j][i+1]-2.0*u[$j][i]+u[$j][i-1])/($dx*$dx)) | ||
| else | ||
| j = findfirst(x->x==input.op, lhs_deriv_depvars) | ||
| expr = :(u[$j][i]) | ||
| end | ||
| return expr | ||
| elseif input.op isa Differential | ||
| grade += 1 | ||
| return discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| elseif input.op isa typeof(*) | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| expr2 = discretize_2(input.args[2],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(:call,:*,expr1,expr2) | ||
| elseif input.op isa typeof(/) | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| expr2 = discretize_2(input.args[2],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(:call,:/,expr1,expr2) | ||
| elseif input.op isa typeof(-) | ||
| if size(input.args,1) == 2 | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| expr2 = discretize_2(input.args[2],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(:call,:-,expr1,expr2) | ||
| else #if size(input.args,1) == 1 | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(:call,:*,:(-1),expr1) | ||
| end | ||
| elseif input.op isa typeof(+) | ||
| if size(input.args,1) == 2 | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| expr2 = discretize_2(input.args[2],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(:call,:+,expr1,expr2) | ||
| else #if size(input.args,1) == 1 | ||
| expr1 = discretize_2(input.args[1],grade,order,dx,lhs_deriv_depvars,lhs_nonderiv_depvars) | ||
| return Expr(expr1) | ||
| end | ||
| end | ||
| end | ||
| 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.
All of this code is quite unnecessary. It should just be a call to CenteredDifference and UpwindDifference
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.
The aim of this algorithm is to traduce an rhs such as:
ln( v(t,x)^Dx(phi(t,x)) / (u(t,x)*(1-u(t,x))) )
where, v(t,x) = 1+2*t*x
to an expression like this:
ln( (1+2*t*i*0.025)^((u[1][i]-u[1][i-1])/0.025) / (u[1][i]*(1-u[1][i])) )
where x=i * dx=i * 0.025, and "u" stores each variable (phi has index 1).
The obtained expression is then evaluated at each time step in the function "f(du,u,p,t)". Using this strategy I avoid using matrices, which is convenient in non-linear problems, and I store every variable within "u", which is consistent with the "f" interface. How would you translate the rhs that I gave using CenteredDifference?
On the other hand, last month I have been thinking how to reimplement this prototype code in a simpler way. Here is an alternative:
Assuming lhs has expressions of this form: Dx(phi(t,x)) or v(t,x)
For each rhs (e.g. Dx(v(t,x)*phi(t,x))) associated to an lhs variable (e.g. phi)
- expand derivatives (does mtk provide this feature?)
e.g.: Dx(v(t,x)*phi(t,x))
=> Dx(v(t,x))*phi(t,x)+v(t,x)*Dx(phi(t,x))
- replace derivatives and variables with its discretizations
e.g.: Dx(v(t,x))*phi(t,x)+v(t,x)*Dx(phi(t,x))
=> ((u[1][i]-u[1][i-1])/0.025)*u[2][i]+u[1][i]*((u[2][i]-u[2][i-1])/0.025)
- simplify obtained expression (does mtk provide this feature?)
- solve the ODE problem
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.
to an expression like this:
You don't want the expression though, since with that form you cannot hit the stencil compiler. This is the reason for the operator form since otherwise you will not have speed or GPU-compatibility.
How would you translate the rhs that I gave using CenteredDifference?
That's an upwind operator call and then a broadcast expression.
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.
You don't want the expression though, since with that form you cannot hit the stencil compiler. This is the reason for the operator form since otherwise you will not have speed or GPU-compatibility.
The whole code is based on building the discretizations as expressions that can be evaluated within f(du,u,p,t), avoiding expressions implies reimplementing many things.
That's an upwind operator call and then a broadcast expression.
Could you please explicitly write the outcome you expect to get when an Operation like the one below is discretized?:
ln( v(t,x)^Dx(phi(t,x)) / (u(t,x)*(1-u(t,x))) )
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.
Dx(phi(t,x)) would be a forward difference operator, and the rest would be a broadcast expression
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.
Dx(phi(t,x)) would be a forward difference operator,
That I had understood.
and the rest would be a broadcast expression
Here is where I do not follow you, would you please write a single line of code with the discretized rhs, using a broadcast expression and the UpwindDifference operator?
rhs = ln( v(t,x)^Dx(phi(t,x)) / (u(t,x)*(1-u(t,x))) )
discretized_rhs = ...
This code should discretize the derivative, with UpwindDifference, but also the other variables: v and u. Furthermore, this discretized code (discretized_rhs) should be evaluated w.r.t. t at each time iteration, that is one of the reasons I used expressions to build the discretization.
| function get_bcs(bcs,tdomain,domain) | ||
| lhs_deriv_depvars_bcs = Dict() | ||
| n = size(bcs,1) | ||
| for i = 1:n | ||
| var = bcs[i].lhs.op | ||
| if var isa Variable | ||
| if !haskey(lhs_deriv_depvars_bcs,var) | ||
| lhs_deriv_depvars_bcs[var] = Array{Expr}(undef,3) | ||
| end | ||
| if isequal(bcs[i].lhs.args[1],tdomain.lower) # u(t=0,x) | ||
| lhs_deriv_depvars_bcs[var][1] = Expr(bcs[i].rhs) | ||
| elseif isequal(bcs[i].lhs.args[2],domain.lower) # u(t,x=x_init) | ||
| lhs_deriv_depvars_bcs[var][2] = :(var=$(bcs[i].rhs.value)) | ||
| elseif isequal(bcs[i].lhs.args[2],domain.upper) # u(t,x=x_final) | ||
| lhs_deriv_depvars_bcs[var][3] = :(var=$(bcs[i].rhs.value)) | ||
| end | ||
| end | ||
| end | ||
| return lhs_deriv_depvars_bcs | ||
| 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.
This should be building a Q operator?
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.
It is a possibility, we should take into account that the BCs should be evaluated w.r.t. t at each time step within "f(du,u,p,t)".
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.
Yes, Q is lazy so you'd just create the type on the fly before evaluation.
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.
Ok. I assume this is planned for an upcoming PR specializing in BCs, is that right?
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.
That should already work
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.
Note: the function get_bcs works, the Q operator is created below in the code:
### Get boundary conditions ################################
# TODO: generalize to N equations
lhs_deriv_depvars_bcs = get_bcs(pdesys.bcs,tdomain,domain[1])
t = 0.0
interior = domain[1].lower+dx[1]:dx[1]:domain[1].upper-dx[1]
u0 = Array{Float64}(undef,length(interior),length(discretization))
Q = Array{RobinBC}(undef,length(discretization))
i = 1
for var in keys(discretization)
bcs = lhs_deriv_depvars_bcs[var]
g = eval(:((x,t) -> $(bcs[1])))
u0[:,i] = @eval $g.($interior,$t)
u_x0 = eval(bcs[2])
u_x1 = eval(bcs[3])
Q[i] = DirichletBC(u_x0,u_x1)
i = i+1
end
|
It looks like tests are failing? |
|
Let's have a call to get in sync. |
I download the github code, at least in my notebook, the tests associated with this code are working fine. |
https://travis-ci.com/github/SciML/DiffEqOperators.jl/builds/180792060#L602-L613 The failure is in the new code so I think it must be related. |
This is the method of lines discretization scheme If you invoke MOLFiniteDifference this way: Then But if you invoke MOLFiniteDifference this way: Then This produced the error when parsing dxs. Any idea about how to modify MOLFiniteDifference to avoid the Tuple case? |
|
Oh, that needs to be an inner constructor then: struct MOLFiniteDifference{T} <: DiffEqBase.AbstractDiscretization
dxs::T
order::Int
MOLFiniteDifference(args...;order=2) = new{typeof(args)}(args,order)
end |
This way makes dxs=(0.1,2) and order=2. I added a small change so dxs is not a tuple: |
| elseif deriv_order == 1 | ||
| # TODO: approx_order and forward/backward should be | ||
| # input parameters of each derivative | ||
| approx_order = 1 | ||
| L = UpwindDifference(deriv_order,approx_order,dx[i],len[i]-2,-1) | ||
| expr = :(-1*($L*Q[$j]*u[:,$j])) | ||
| elseif deriv_order == 2 |
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.
The difference here isn't 1 or 2, but more generally, when it's odd you want to use upwinding and when it's even you want to use centered difference.
| # TODO: discretize the following cases | ||
| # | ||
| # 1) PDE System | ||
| # 1.a) Transient | ||
| # There is more than one indep. variable, including 't' | ||
| # E.g. du/dt = d2u/dx2 + d2u/dy2 + f(t,x,y) | ||
| # 1.b) Stationary | ||
| # There is more than one indep. variable, 't' is not included | ||
| # E.g. 0 = d2u/dx2 + d2u/dy2 + f(x,y) | ||
| # 2) ODE System | ||
| # 't' is the only independent variable | ||
| # The ODESystem is packed inside a PDESystem | ||
| # E.g. du/dt = f(t) | ||
| # | ||
| # Note: regarding input format, lhs must be "du/dt" or "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.
Note that it shouldn't always be an ODEProblem. If there isn't a "preferred dimension", then it should become a NonlinearProblem. We should discuss this a bit more.
| # input parameters of each derivative | ||
| approx_order = 1 | ||
| L = UpwindDifference(deriv_order,approx_order,dx[i],len[i]-2,-1) | ||
| expr = :(-1*($L*Q[$j]*u[:,$j])) |
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.
yup that's what I had in mind, basically. Though the way you're writing it is going to allocate and it's not going to extend to higher dimensions. We should talk next about how to get to higher dimensions.
| bcs = lhs_deriv_depvars_bcs[var] | ||
|
|
||
| g = eval(:((x,t) -> $(bcs[1]))) | ||
| u_t0[:,j] = @eval $g.($(X[2][2:len[2]-1]),$t) |
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.
This should make use of https://github.com/SciML/RuntimeGeneratedFunctions.jl
|
This looks good to merge. I made a few comments on things I saw that need to change, but they can come in follow ups and some of them are very simple. Thanks! |
|
🎉 |
|
Great! Thanks for the comments, I will take them into account for the next PR :-) |
|
I opened an issue per comment to keep track of them. |
Excellent |
One-dimensional PDE auto-discretization using finite-difference (work in progress)
- Typical cases of 1D diffusion-convection equation:
- [x] Dt(u(t,x)) ~ Dxx(u(t,x))-vDx(u(t,x))
- Source/sink term
- [x] All cases above support a constant source/sink term in rhs
- [x] All cases above support a source/sink term which depends on (t,x) in rhs
- Boundary conditions
- [x] All cases above support Dirichlet boundary conditions