aresio / simpful

A friendly python library for fuzzy logic reasoning
Academic Free License v3.0
130 stars 33 forks source link

Incorrect plotting in FuzzySystem plot_surface method #30

Open guilherme-marcello opened 1 month ago

guilherme-marcello commented 1 month ago

Hello there,

It seems there is a bug in the plot_surface method of the FuzzySystem class that results in an incorrect surface plot.

In order to make the problem more evident, I changed slightly the code example provided in https://github.com/aresio/simpful/blob/master/examples/example_output_surface.py . The following code reproduces the issue:

from simpful import *
import matplotlib.pylab as plt
from numpy import linspace, array

FS = FuzzySystem()

S_1 = FuzzySet(function=Triangular_MF(a=0, b=0, c=5), term="poor")
S_2 = FuzzySet(function=Triangular_MF(a=0, b=5, c=10), term="good")
S_3 = FuzzySet(function=Triangular_MF(a=5, b=10, c=10), term="excellent")
lvarS = LinguisticVariable([S_1, S_2, S_3], concept="Service quality", universe_of_discourse=[0,10])
FS.add_linguistic_variable("Service", lvarS)

F_1 = FuzzySet(function=Triangular_MF(a=0, b=0, c=10), term="rancid")
F_2 = FuzzySet(function=Triangular_MF(a=0, b=10, c=10), term="delicious")
lvarF = LinguisticVariable([F_1, F_2], concept="Food quality", universe_of_discourse=[0,10])
FS.add_linguistic_variable("Food", lvarF)

T_1 = FuzzySet(function=Triangular_MF(a=0, b=0, c=10), term="small")
T_2 = FuzzySet(function=Triangular_MF(a=0, b=10, c=20), term="average")
T_3 = FuzzySet(function=Trapezoidal_MF(a=10, b=20, c=25, d=25), term="generous")
lvarT = LinguisticVariable([T_1, T_2, T_3], universe_of_discourse=[0,25])
FS.add_linguistic_variable("Tip", lvarT)

R1 = "IF (Service IS excellent) THEN (Tip IS generous)"
FS.add_rules([R1])

fig = FS.plot_surface(variables=['Service', 'Food'], output='Tip', detail=5)

The relevant change here is that a single rule "IF (Service IS excellent) THEN (Tip IS generous)" was added to the fuzzy system.

Observed output: The resulting plot (image bellow) incorrectly suggests that the tip increases with food quality instead of service quality.

Figure_1

The version of the packages I used in this execution:

I also replicated the issue with the following versions:

Is this really an unexpected behavior?

Thanks. Guilherme.

guilherme-marcello commented 1 month ago

After reviewing the issue in detail and exploring the relevant documentation, I have identified the cause of the problem and a few potential solutions.

The issue with the plot_surface method arises from a mismatch between how the surface data (C) is constructed and how numpy's meshgrid is used to generate the grid coordinates.

Specifically, the method currently constructs the surface data C such that each row of C corresponds to a fixed value of the first variable (e.g., service quality), while the meshgrid function (with Cartesian indexing, the default indexing='xy') creates grids where this relationship is inverted. This leads to an incorrect mapping of variables in the plot, causing the surface to incorrectly depend on the wrong axis - as presented in the attached image.

Cause of the issue

The problematic loop in the current code is as follows:

for a in A:
    temp = []
    for b in B:
        self.set_variable(v1, a)
        self.set_variable(v2, b)
        res = self.inference()[output]
        temp.append(res)
    C.append(temp)
C = array(C)
A, B = meshgrid(A, B)

Here, as described above, the variable C is constructed with the assumption that each row corresponds to a constant value a of v1 (e.g., service quality) and each column corresponds to a constant value b of v2 (e.g., food quality). However, meshgrid (with Cartesian xy indexing) swaps this arrangement when generating the A and B grids for plotting. So, as a result, plot_surface plots the surface incorrectly.

Possible solutions

I found three ways to fix it, that with some testings seemed to be reasonable.

Change the indexing in `meshgrid``

We can fix the issue by changing the indexing mode in the meshgrid function to match how C is constructed. Specifically, setting indexing='ij' will use matrix indexing, which aligns with how the C array is populated.

The change would be:

A, B = meshgrid(A, B, indexing='ij')

Change how C is constructed:

Alternatively, we could modify the loop that constructs C to reflect the Cartesian (xy) indexing used by meshgrid. This would involve swapping the roles of the variables inside the loop to ensure that C is built consistently with Cartesian indexing.

We could rewrite the loop as follows:

for b in B:
    temp = []
    for a in A:
        self.set_variable(v1, a)
        self.set_variable(v2, b)
        res = self.inference()[output]
        temp.append( res )
    C.append(temp) 

Transpose C

Another simple fix is to transpose the C array after it has been constructed. This approach corrects the mismatch between the way C is built and how meshgrid expects the grid values to align, without needing to modify meshgrid's indexing mode. The added code in this fix is:

C = array(C)
C = C.T 

Recommended fix

While the solutions are valid, I would recommend the first one - changing the meshgrid indexing to 'ij' - since it involves fewer changes to the existing code and preserves the logic flow in plot_surface.

Please let me know if more information is required, and I would be happy to assist with further details or testing.

Guilherme.