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
Create fuzzy_quantifiers #12
base: master
Are you sure you want to change the base?
Conversation
Location: fuzzy-rough-learn\frlearn\uncategorised
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12 +/- ##
=======================================
Coverage 82.75% 82.75%
=======================================
Files 44 44
Lines 1357 1357
=======================================
Hits 1123 1123
Misses 234 234 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some initial comments. I will add some more comments later, but you can start by addressing these.
If you push a new commit to this branch, your changes will automatically be added to this pull request.
Thank you for also creating an example, but please add that file to this pull request (by pushing to this branch).
A, B: table (list) | ||
List of value of the Fuzzy sets | ||
""" | ||
return rim(sum([el for el in A if el in B]) / sum(A)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? Wouldn't A and B be two lists of values in [0, 1]
that are generally not identical, and for which you want to compare the values in corresponding positions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes of course. I want to compute the sum of elements which belongs in the intersection of A and B, so i write sum([x for x in A if x in B])
return zadeh_function(0.1, 0.4, p) | ||
|
||
|
||
def z_eval_unary_sentence(set_v, rim, A): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I don't think you need
set_v
, the only information that you need is the size of the base set X, which you can get aslen(A)
. - Please spell out 'zadeh' in the function name, e.g.
unary_zadeh_model
. Same for the binary function below, and the yager models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, set_v is never needed, also for the Choquet integral it is not necessary. Any function f we want to integrate can be represented as a simple list (just like A in this function).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed! I've corrected it.
return choquet_integral(set_v, f, mu) | ||
|
||
|
||
def y_eval_binary_sentence(set_v, f, rim, A, B=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this function down here, and not up with the corresponding unary function and the Zadeh functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I wrote this code, I followed the order of the definitions of fuzzy quantificators in the paper 'Fuzzy rough sets based on fuzzy quantification, Adnan Theerens ∗, Chris Cornelis'. But now I've but in back in the right place.
elif (a + b) / 2 <= p <= b: | ||
return 1 - ((2 * (p - b) ** 2) / (b - a) ** 2) | ||
else: | ||
return 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This seems to be the same class of quantifiers as the
QuadraticSigmoid
class inunclassified.quantifiers
, so you can remove this definition. - I am not sure whether we should define
most
andsome
here, since different users may want to use different thresholds. So you could move these to the example. - In terms of
QuadraticSigmoid
, this would becomemost = QuadraticSigmoid(0.3, 0.9)
. Since the resulting object is callable, you can then usemost(p)
to evaluatep
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly! I noticed it.
|
||
|
||
def for_all(p): | ||
return at_least(1, p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move more_than
, exists
, at_least
and for_all
to the existing unclassified.quantifiers
module, and define more_than
and at_least
analogously to QuadraticSigmoid
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
parameters: | ||
---------- | ||
set_v: table (list) | ||
The base set X on which the mathematics function f is defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that it makes sense to take set_v
as an input. Your code seems to implicitly assume that set_v
consists of (possibly identical) values, which isn't the same as the underlying set, and such values may not always be present? How about instead requiring two corresponding lists f_val
and mu_val
, which represent the values of f
and mu
applied to the elements of the underlying set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we can use two lists like f_val and mu_val as parmeters of this function, it will work correctly, but, for example, to evaluation a binary sentence using the Yager's model, we have to write many lines of code to get the differents values of the monotone measure. On the other hand, if we took the monotone measure to be a function and set_v to be a parameter, things would be clearer, I think. In fact, this is what motivated me to write the code as follows.
|
||
def A_min(gamma, A, set_v): | ||
"""" | ||
The sets A_min of Definition 2.17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should provide a reference to where this Definition 2.17 is to be found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!
@amtheere Could you have a look at this code, to check whether the implementation is correct? |
def y_eval_unary_sentence(set_v, rim, A): | ||
""" | ||
Evaluation of unary sentences of the form “Λ X’s are A’s” in Yager’s OWA model, where Λ is a RIM quantifier | ||
parameters: | ||
---------- | ||
rim: function | ||
Definition of the RIM quantifier | ||
A: function | ||
Function defining the fuzzy set A | ||
set_v: table (list) | ||
The base set X on which the fuzzy set is defined | ||
""" | ||
def mu(x): | ||
return rim((len(x)/len(set_v))) | ||
return choquet_integral(set_v, A, mu) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an OWA operator, so defining a measure and then using the general choquet_integral function is inefficient. Same thing for other functions, for specific measures you can simplify the choquet integral.
Also remove set_v and define A as a list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just implemented the definitions of quantificators present in the parper 'Fuzzy rough sets based on fuzzy quantification, Adnan Theerens ∗, Chris Cornelis'. As for the efficiency of these quantificators, I haven't thought about it.
def W_implicator(set_v, f, rim, A, B=None): | ||
"""" | ||
Evaluation of a binary sentence using a fuzzy quantifier based | ||
on a Weighted Ordered Weighted Averaging (WOWA) | ||
:parameter | ||
set_v: table (list) | ||
The base set X on which the fuzzy sets are defined | ||
f: function | ||
Define the Implicator | ||
rim: function | ||
define the rim quantifier | ||
A, B: table (list) | ||
List of the values of the fuzzy sets A and B | ||
""" | ||
def mu(x): | ||
return rim(sum([el for el in x if el in A]) / sum(A)) | ||
return choquet_integral(set_v, f, mu) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right, you define f as the implicator function (which is confusing, rename it) and then pass it directly to the Choquet integral function as the to be integrated function. Or did you mean that f already contains I(A,B)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in this function, f ix exactly I(A,B). I renamed it and made the requested changes
|
||
|
||
def y_eval_binary_sentence(set_v, f, rim, A, B=None): | ||
"""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected.
Location: fuzzy-rough-learn\frlearn\uncategorised