Davide-sd / sympy-plot-backends

An improved plotting module for SymPy
BSD 3-Clause "New" or "Revised" License
42 stars 9 forks source link

Interactive plots - parameters should be of type complex and not float #17

Closed Davide-sd closed 1 year ago

Davide-sd commented 1 year ago

During evaluation of interactive series classes, all parameters should use a complex number instead of float. For example, the following is going to fail:

from sympy import *
from spb import *
init_printing()
xi, wn, x0, v0, t = symbols("xi, omega_n, x0, v0, t")
x = Function("x")(t)
eq = x.diff(t, 2) + 2 * xi * wn * x.diff(t) + wn**2 * x
sol = dsolve(eq, x, ics={x.subs(t, 0): x0, x.diff(t).subs(t, 0): v0})
plot(
    sol.rhs, (t, 0, 100),
    params={
        wn: (0.5, 0, 6*pi),
        xi: (0.25, 0, 1),
        x0: (0.45, 0, 4),
        v0: (0, 0, 4)
    },
    backend=PB
)

Here, the problem is the xi uses a float value, but xi < 1, thus sqrt(xi**2 - 1) will evaluate to NaN. Using complex numbers fixes the problem.

michelececcacci commented 1 year ago

is this still up for taking?

Davide-sd commented 1 year ago

Sure, go ahead!

michelececcacci commented 1 year ago

Small update: i wrote a really simple fix (just casting to complex any parameter that is either float or int in the InteractiveSeries constructor) and the warning is gone, but test_complex_adaptive_false (line 3014) fails (because some elements are not in np.allclose() 's default range ). Is this expected?

Davide-sd commented 1 year ago

I've been able to replicate the error.

Within InteractiveSeries class (and its subclasses), the self._params attribute receives data whenever a widget changes state (a slider gets moved, ...). So, I would probably do the casting inside the _evaluate method, or inside the param property getter.

Now to the error. The x symbol represents the discretized range. Apparently, I decided to evaluate the function using a real discretized range, instead of a complex one (at the moment, the reason eludes me...). Which means that inside test_complex_adaptive_false, s1 will produces wrong data.

That's another bug: #19 .

Feel free to apply a PR for this, even if it raises errors it doesn't matter, because the main issue is #19.