-
-
Notifications
You must be signed in to change notification settings - Fork 6
Prototype for the first target (#33) #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Can you show me the interface for how to use them? I am less interested in the backend than the user interface at this point. |
dx = step(xgrid) | ||
if dorder == 2 && aorder == 2 | ||
L = UniformDiffusionStencil(M, dx) | ||
QB = QRobin(M, dx, BC.al, BC.bl, BC.ar, BC.br) |
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 see this being a build_Q
dispatching on BC types? Robin
, Dirichlet
, Neuman
? The denominators would require such handling
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.
Yep that's the plan. Also we should divide the BC into left/right parts as we currently do in DiffEqOperators.
operator_examples/first_target.jl
Outdated
|
||
# TODO: stencil operators for irregular grids | ||
|
||
struct QRobin <: AbstractDiffEqLinearOperator{Float64} |
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 Q works for any local operator. Non-local operators, like jump operators, need to have a bunch of vectors in here. Also, this is all based around using *
instead of mul!
. I am getting a feeling the mul!
usage is much more difficult with this approach.
bc = (al = 0, bl = 1, ar = 0, bl = 1)
Dx = DerivativeOperator(0:dx:1,1,1,bc)
Dxx = DerivativeOperator(0:dx:1,1,1,bc)
L = Dx + Dxx
full(L) \ u |
What's the plan for scalars here? I think that instead of |
I agree that upwind should be the default, and not But I realized here that it may not be that simple with composition. For example, lets say that I want to use upwind on an operator with negative drift.
We know that if Then lets say I am solving the stationary HJBE equation
with To solve this, we rearrange and define a new operator,
So in that case with the composition, it seems to work if the But now lets say that instead of composing things like that did the distributed operation ourselves
or even
All three of these should be equivalent by linearity of the operator, as we know that for Furthermore, are there cases where instead of the default of Not that these issues are unsurmountable, but I think they should be thought through prior to implementing upwind. |
I guess it also depends on how |
These confusions are all very much related to a bad choice. The choice shouldn't be default upwind or anything like that, it should just know how to handle a coefficient with a default
Yes. If you just embed a coefficient in there, defaulting to |
That's basically the gist of it. For unitary But I agree that we should be extra careful with how upwind operators & coefficients (scalar/vector) are handled. Meanwhile @jlperla do you think the |
I think so. My assumption is that we are going to layer sugar on top of this in a near future step (before I show off the code) , but I think I see how that would work. We can discuss later, but shared sugar in some way with ApproxFun.jl is preferred. |
Let's get it working before we start talking about sugar and DSLs. I always like to have a fully functional interface underneath. |
OK. I think I understand the design now, where the coefficients are attached to the operator, but users don't necessarily construct them that way. And maybe the upwind based on the coefficient sign is always correct. However, I also think that we should also consider operators which force backwards or forwards differences at some point... Even if they are ultimately in separate types. But for my immediate needs (which are consistent based on the scalar sign to what is here) your suggested implementation will always work. |
After working on a few prototypes I think I've hit a bottleneck for the upwind operators: the need to know the coefficients before determining the upwind direction and the fact that DiffEqOperators' composition is lazy. Consider this artificial example: L = c1(t) * (c2(t)L1 + L2) where The problem is that the composition is lazy, so however we build the operators, we always have One way out of this is to discard total laziness and distribute scalar/vector coefficient multiplitcation across a L = (c1(t)*c2(t))L1 + c1(t)L2 Getting two time-dependent |
Yes this example is exactly what I was getting at with my concerns on the implementation... My feeling is that we should only have the upwind sign determined by the sign of the immediate left multiplication. That is easy to describe to people, and they can use brackets to manually change signs if they want to. Precedence of multiplication over addition helps us here |
This seems like an acceptable compromise to me, which should be able to avoid complex tree transversals while keeping the lazy compositions in tact. |
We probably need a few iterations before arriving at the final decision. But first of all we should agree that the intended user interface should look like LB1 = DerivativeOperator(xgrid, 1, 1, BC)
LB2 = DerivativeOperator(xgrid, 2, 2, BC)
LB = mu * LB1 + sigma * LB2 That is, the coefficient (either as a scalar, vector or a Now onto my current prototype: the gist is that a The problem with this approach is that the stencil operator |
I would be interested to get thoughts from @ChrisRackauckas but I am not sure it is a compromise. Having the drift direction determined by the immediate sign may actually be the "right" way to do it. It certainly makes my concerns about #34 (comment) more predictable, as you can setup the theory to make sure upwind occurs correctly, and not be surprised if the direction changes just because you start composing stuff. |
Oh I haven't thought of this possibility. BTW I'm currently working on an Option 2 which is pretty similar to #34 (comment) but instead of |
I've put Option 2 on a separate branch to make comparisons easier: The difference is that the embedded coefficient is handled on the stencil level with |
Prototype code for #33. The focus is on setting up the matrix-free operators for the generic
L*QB
route and come up with a mock user interface to create the operators, all the while taking composition and irregular grid into account.The basic idea I have is to separate the codebase into two parts: the low level operators and the high-level method
DerivativeOperator
. The user will never need to explicitly construct any of the low level operators, but instead use the interface defined byDerivativeOperator
, which is guaranteed to return a derivative operator or a composed derivative operator (a "factory method" in OOP jargon).The current mock
DerivativeOperator
interface looks very much the same as DiffEqOperator's current implementation, with the difference being the presence ofxgrid
serving to differentiate between uniform and irregular grids. Also I wrote aDerivativeOperator
with noBC
as the factory method to construct stencil operators (or we can use a different name if this is confusing).However this version of the interface has the same issue as pointed out by #33 (comment), i.e. by packaging the construction of square derivative operators in one place we end up unable to do
LB = (L1 + L2)*QB
but only the distributed versionLB = L1*QB + L2*QB
. Of course now that we can also construct stencil operators independently, we can add another user interface for constructingQB
(as well as utilities for extending/zero padding the Ls) and then the first version is also possible. Or we can rebuild an entirely new interface.@jlperla I'd like to hear your opinion on the prototype. Meanwhile I'll try writing an example script to demonstrate the operators' usage.