maroba / findiff

Python package for numerical derivatives and partial differential equations in any number of dimensions.
MIT License
420 stars 60 forks source link

Possible interface improvements #4

Closed ozankabak closed 6 years ago

ozankabak commented 6 years ago

Hi! I recently discovered your package and I think it is great. I have a few suggestions about the API. Consider this example:

d_dx = FinDiff(h=[dx, dy, dz, du], dims=[0])
df_dx = d_dx(f)

Do you really need [dy, dz, du] if dims is equal to [0]? Consider the following interface

diff_result = FinDiff(f, (dim1, delta1, count1), (dim2, delta2, count2), ...)

where the last count fields are optional with a default value of 1. For example,

df_dx = FinDiff(f, (0, dx))
df_dx2 = FinDiff(f, (0, dx, 2))
df_dxdy = FinDiff(f, (0, dx), (1, dy))

Also, if one needs a callable, it is always easy to use a lambda:

d_dx = lambda x: FinDiff(x, (0, dx))

Finally, to support non-uniform grids, you can check whether the delta field is a scalar or a vector.

I have a few other suggestions, but I'm curious to hear what you think of these first.

Again, thanks for publishing this package. I think it is weird that NumPy doesn't have this built-in, and this package fills an important gap. Maybe it will eventually find its way into NumPy.

maroba commented 6 years ago

Hi!

Thank you very much for suggestions, I really appreciate that! Yes, I also wondered why neither numpy nor scipy have a dedicated module for finite differences. Ok, finite differences are easy to implement, but I have done that over and over again just to try something out, so why not have some convenience package for that purpose?

Regarding the API I think you are absolutely right. I'm not satisfied with the current API, either. What disturbs me most is the "dims" parameter. How could anyone guess that "dims" means the dimensions along which the derivatives are to be taken? But I couldn't think of a better name. ;-) Your solution is much clearer and also what users of Mathematica or sympy would expect.

For the uniform grids, I completely agree with your solution. For the case of non-uniform grids, I'm not sure if simply replacing the scalar dx by a vector dx is really convenient. I'm wondering what the user normally has available before calling the API. Is it a vector of coordinates or a vector of grid spacings? If it's a vector of grid spacings, then fine. If it's a vector of coordinates of length N, it may be a bit inconvenient to construct the vector of spacings of length N-1 before calling the API. And then there is problem of directions: does dx[i] mean x[i+1]-x[i] or x[i]-x[i-1]? What do you think?

In any case, I think your suggestion is absolutely the right direction for the API, and I would be happy to hear your other suggestions.

ozankabak commented 6 years ago

For the case of non-uniform grids, I'm not sure if simply replacing the scalar dx by a vector dx is really convenient.

I actually agree with this. Changing the semantics of the dx field in the non-uniform case is somewhat awkward. It enables one to use a single function for both uniform and non-uniform cases, but loses clarity.

In the non-uniform case, I think most users will have grid points at hand, not differences; therefore the API probably should be tailored towards this use case. One idea is to have two functions, one for the uniform case and one for the non-uniform case (say, findiff_uniform vs. findiff_irregular). The signatures would look similar to what I proposed but the second element of the tuple would be a vector of coordinates in the latter case. The semantics would be clear from the function names (and the argument names). This approach is also compatible with how NumPy generally behaves: They tend to use distinct functions instead of trying to infer the intent from the arguments (e.g. see dot, vdot, tensordot).

BTW, I will open separate threads of discussion for other possible suggestions. I want to get some more practical usage of FinDiff before doing that, though.

maroba commented 6 years ago

Ok, I have created a branch "new-api" where I have pushed changes implementing a new API for the class FinDiff.

For uniform grids, the standard way would be to use

d_dx = FinDiff((0, dx))
d2_dx2 = FinDiff((0, dx, 2))
d2_dxdy = FinDiff((0, dx), (1, dy))

I think for non-mixed derivatives, it would be nicer to get rid of the inner tuple, so that I can simply say

d_dx2 = FinDiff(0, dx, 2)

Specifying the accuracy order requires to give the keyword argument "acc":

d2_dxdy = FinDiff((0, dx), (1, dy), acc=4)

A big plus for the new API is that it is not necessary to give all spacings, but just the ones needed for the required derivatives. And one and the same FinDiff objects can be used to differentiate arrays of different dimensions.

For non-uniform grids I still use the same class FinDiff, but now with different arguments:

d_dx = FinDiff((0, 1), coords=[x, y])
d2_dy2 = FinDiff((0, 2), coords=[x, y])

I'm thinking about extracting the non-uniform stuff into its own class like FinDiffNonUniform, but I'm still hesitating. For me "FinDiff" means "finite difference", and that says nothing about uniform or not. I see the point that inferring the intent by the arguments is not really nice, but with the new API I think the intent can at least be stated a bit clear. What do you think? BTW, under the hood, FinDiff already uses two different classes BasicFinDiff and BasicFinDiffNonUniform by composition.

ozankabak commented 6 years ago

The more I think about it, the more I'm convinced that the "shape" of a second tuple element is actually a clear signal of intent. To capture this intent; however, you will need to present a NumPy-esque interface instead of returning a callable. Consider:

df_dj = FinDiff(f, (j, u))

Here are the possibilities:

If you want extra security, you can add a keyword argument mode whose default value would be auto, which means the above rules are applied to infer intent. A cautious programmer will then be able to manually provide a value for mode if s/he wants to program defensively.

BTW, giving up on returning callables will also obviate the need for a custom Coefficient class. If I want to make a linear combination of finite differences, I can easily write a one-liner lambda for that and use simple Python constants, no need for a Coefficient.

maroba commented 6 years ago

I guess I should explain, why I decided to make FinDiff a callable. The point is that for non-uniform grids, the constructor itself is not inexpensive. It must solve many (small) linear equation systems, one for each coordinate, to determine the finite difference coefficients, which are position dependent for non-uniform grids. So I wanted the FinDiff object to represent an operator that can be re-used and applied to different arrays many times. This would not be the case, if the derivative is already performed in the constructor, as suggested by

df_dj = FinDiff(f, (j, u))

So I decided to separate it into two steps, construction and application. Finally, I didn't want to do something verbose like d_dx.apply(f), so I chose the callable.

But the more I think about it (or the more I get used to it? ;-) ), the more I agree that allowing the second tuple element to represent grid coordinates for non-uniform grids is not confusing. So I would propose to stay with the callable approach, but follow your idea to allow the second argument be a vector of coordinates or spacings and infer which is which by the shape.

The need for the custom Coefficient doesn't arise from FinDiff being a callable. In fact, you can multiply FinDiff objects from left or right by Python constants without wrapping them in a Coefficient object. The need arises from multiplying FinDiff objects by numpy.ndarrays. For instance, when you have

x, y, z = [np.linspace(...), ...]
X, Y, Z = np.meshgrid(x, y, z, indexing='ij')

you cannot say

X * FinDiff(0, dx)

because Python finds a __mul__ method in the numpy.ndarray class for X and tries to use that for multiplication. So Python does not look for the __rmul__ method in FinDiff. But numpy.ndarray does not know how to multiply by a FinDiff object, so it throws an exception. What would work is

FinDiff(0, dx) * X

because FinDiff has its own __mul__ method and knows how to multiply a numpy.ndarray. But since FinDiff is a differential operator, the multiplication with X is not commutative, so that's not an option anyways. So, the reason why I introduced the Coefficient class is that I wanted to control the __mul__ behavior both for numbers and for arrays.

ozankabak commented 6 years ago

The point is that for non-uniform grids, the constructor itself is not inexpensive. It must solve many (small) linear equation systems, one for each coordinate, to determine the finite difference coefficients, which are position dependent for non-uniform grids. So I wanted the FinDiff object to represent an operator that can be re-used and applied to different arrays many times.

This is an important point. There are other possible ways to accommodate this requirement without using a callable, but they'd have their own downsides.

So I would propose to stay with the callable approach, but follow your idea to allow the second argument be a vector of coordinates or spacings and infer which is which by the shape.

I think this is a reasonable path forward!

The need for the custom Coefficient doesn't arise from FinDiff being a callable ...

I'm not sure if supporting an algebra involving callables is a good idea. In theory, it would be great to exactly mimic common mathematical notation, but it seems to me that the confines of Python as the host language ends up making everything more cumbersome than otherwise. Consider the two cases where we want to compose operators op1 = x * d_dx and op2 = d_dx * x. Instead of fighting Python, if we embrace its facilities, we can say:

op1 = lambda u: x * d_dx(u)
op2 = lambda u: d_dx(x * u)

In general, we'd use a lambda with N arguments whenever we'd like to compose an operator with N arguments. The intent is very clear and the code is succinct. I don't see how using Coefficients (which sometimes need explicit construction) and overloading operators make things more succinct or clear. I think this is one of the times where not having a feature provides a value by constraining the user to follow a "safer" route.

maroba commented 6 years ago

Ok, I have pushed changes with the new API for non-uniform grids. So far, it's only possible to give coordinate arrays, but not yet spacings:

x = #... irregular 1D array
diffop = FinDiff(0, x, 2)

I'm also not satisfied with the Coefficient type, but mostly because of asymmetry.

I don't see how using Coefficients (which sometimes need explicit construction) and overloading operators make things more succinct or clear.

I'm not sure if using lambdas really makes things more succint. Consider the following example. Suppose you want to construct the 2D Laplacian in polar coordinates:

r = np.linspace(0.1, 10, 100)
phi = np.linspace(0, np.pi, 100, endpoint=False)
dr, dphi = [c[1] - c[0] for c in (r, phi)]
R, Phi = np.meshgrid(r, phi, indexing='ij')

With lambdas you would say

d_dr = FinDiff(0, dr)
d2_dr2 = FinDiff(0, dr, 2)
d2_dphi2 = FinDiff(1, dphi, 2)
laplace = lambda f: d2_dr2(f) + R**-1 * d_dr(f) + R**-2 * d2_dphi2(f)
laplace(f)

With Coefficient it's a one-liner

laplace = FinDiff(0, dr, 2) + Coefficient(R**-1) * FinDiff(0, dr) + Coefficient(R**-2) * FinDiff(1, dphi, 2)
laplace(f)

I think this is one of the times where not having a feature provides a value by constraining the user to follow a "safer" route.

Fair point. Using Coefficient is really not the safer route, in particular in the situation now where you can only use left multiplication.

ozankabak commented 6 years ago

This was a great discussion, thanks for contributing to it so openly. I leave the final decision on the Coefficient type to you, we've already explored the trade-offs that come with having that type.

maroba commented 6 years ago

It's me who has to say thanks. Your suggestions have made the API a lot clearer.