Skip to content
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

Bug in SmallestOfMaximum? #9

Closed
CJxD opened this issue Mar 13, 2022 · 3 comments
Closed

Bug in SmallestOfMaximum? #9

CJxD opened this issue Mar 13, 2022 · 3 comments

Comments

@CJxD
Copy link

CJxD commented Mar 13, 2022

I was just reading through the defuzzifiers, when I noticed that the code for SmallestOfMaximum is the same as LargestOfMaximum. The only difference is the initial value of x_smallest. I think the for-loop is supposed to go from resolution down to 0 rather than 0 to resolution.

Am I missing something?

    def defuzzify(self, term: Term, minimum: float, maximum: float) -> float:
        if not math.isfinite(minimum + maximum):
            return nan
        resolution = self.resolution
        dx = (maximum - minimum) / resolution
        y_max = -math.inf
        x_smallest = minimum
        for i in range(0, resolution):
            x = minimum + (i + 0.5) * dx
            y = term.membership(x)
            if Op.gt(y, y_max):
                y_max = y
                x_smallest = x
        return x_smallest

for i in range(0, resolution):

@jcrada
Copy link
Member

jcrada commented Mar 13, 2022

Hi Chris,

Thanks for your report.

The code is actually fine. The difference is in the comparison if Op.gt(y, y_max): for smaller of maximum, and if Op.ge(y, y_max): for largest of maximum, that is, Op.gt: greater than and Op.ge: greater than or equal to.

Say you have this:

      ______________
_____/              \_____

The smaller of maximum should get the first (left to right) x coordinate where y is maximum, so you iterate and change only when you find a greater than value.

The largest of maximum should get the last (left to right) x coordinate where y is maximum, so you iterate and change when greater than or equals to.

This is maybe easier to see in QtFuzzyLite, just make sure to use Trapezoid terms instead of Triangle terms.

Does this answer your question?

Cheers.

@jcrada
Copy link
Member

jcrada commented Mar 13, 2022

Oh, I see what you mean, Chris!
Yes, it is actually a bit more efficient to do the same but in reverse 😅.
The iteration would be instead:

            x = maximum - (i + 0.5) * dx

@CJxD
Copy link
Author

CJxD commented Mar 13, 2022

Oh yes, that's a very subtle change to do Ge rather than Gt, didn't notice that! Makes sense.

I'm in the final stages of my "fuzzyverylite" extension after getting a bit more time to have another look. It uses the Python library to parse the FLL, and then spits out C++ code to perform the calculations. I've managed to replicate one of our models in only 23KB of binary so far. I'll send it your way when I'm done with it for academic interest!

Heads up, I noticed in my last push over Christmas that pyfuzzylite couldn't load one of our FLLs successfully. I'll make another bug report if the same still happens.

@CJxD CJxD closed this as completed Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants