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

FIS Exporter: export AlgebraicSum as probor #48

Conversation

sguazt
Copy link
Contributor

@sguazt sguazt commented Jul 25, 2015

Hi,

Currently, the FIS Exporter exports the AlgebraicSum operator as sum.
This is wrong since the MATLAB's counterpart of the AlgebraicSum is the probor operator (see https://www.mathworks.com/help/fuzzy/probor.html).

Also, note that despite MATLAB allows to use the sum operator as a rule aggregation method, it is treated differently from probor (see https://www.mathworks.com/help/fuzzy/fuzzy-inference-process.html#a1054218661b1).

Best,

Marco

@sguazt
Copy link
Contributor Author

sguazt commented Jul 26, 2015

I think fl::FisImporter should be changed as well.
Indeed, at line 373:

if (name == "sum" or name == "probor") return AlgebraicSum().className();

operators sum and probor are treated as the same operator. But I don't think it is correct (see second link to MATLAB doc in my first message).

What do you think?

@jcrada
Copy link
Member

jcrada commented Jul 27, 2015

Hi Marco. Thank you for your contribution. I will check the tests and examples and merge your request if it is all good. Thanks.

@jcrada
Copy link
Member

jcrada commented Aug 11, 2015

Hi Marco, I am sorry for such a long delay to introduce your changes. I agree the export method should provide probor instead of sum. Could you please make a pull request to the develop branch instead such that it is available from the next version onwards?

As for what you mention on FisImporter, I agree to that too. I will create a new issue to add a new UnboundedSum.

@sguazt
Copy link
Contributor Author

sguazt commented Aug 13, 2015

Hi Juan,

Is this pull request already for the develop branch?

Marco

@sguazt
Copy link
Contributor Author

sguazt commented Oct 25, 2015

Hello Juan,

In the last message, you ask me to make a pull request to the develop branch.
AFAIK, this request is already for the develop branch.
Indeed, in the heading (at the beginning of this page) I see:


sguazt wants to merge 1 commit into fuzzylite:develop from sguazt:patch/fis_export-algebraic_sum-as-probor 

Could you tell me what should I do?

Thank you so much.

Marco

@jcrada
Copy link
Member

jcrada commented Oct 25, 2015

Hi Marco, thank you for your message. Please excuse the delay in merging this request. I had been on holidays for two months, then moved to a new city and job last week, and have not had internet at home since.

The merge you are requesting involves simple changes with important implications, which I have not had the time to evaluate. In particular, my main concern is that the changes will change some of the SNorms in the examples, which could change their operation and maybe their results. The results are very important because they validate that the accuracy matches that of Matlab.

It is therefore that I have been merging very simple pull requests, and when I have some time I continue to improve the documentation branch.

I will be busy next week, but will assign top priority to merge and test this request within two weeks.

Thank you for your patience and most of all for your valuable contribution.

@sguazt
Copy link
Contributor Author

sguazt commented Oct 25, 2015

OK, no problem.
I've been busy too. Actually, I'm always too busy ;-)

So, take your time and good luck for your "new life".

Marco

@jcrada jcrada closed this Feb 8, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants