-
Notifications
You must be signed in to change notification settings - Fork 68
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
_quantity() relationship modifications #113
Conversation
@@ -273,7 +273,8 @@ public boolean test_comparatives() | |||
"_obj(run, mile)\n"+ | |||
"_subj(run, he)\n" + | |||
"_subj(do, John)\n" + | |||
"_quantity(mile, many)\n"+ | |||
"_quantity(mile, more)\n"+ | |||
"_quantity_mod(more, many)\n"+ |
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.
@ruiting @linas pls check the "_quantity_mod(more, many)" relationship here.
sent 1- He runs ten more miles than John.
sent 2- He runs many more miles than John.
link-parser output of sent 1 &2 are same. For the sent 1 "_num_quantity(more, ten)" can be used but for sent 2 if I use "_num_quantity(more, many)" it would be wrong, because "many" is not a numeric value. hence I used the "_quantity_mod(more, many)" relationship here. Do you think that is correct?
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.
There is no _num_quantity in the default relex, or, at least, there is not supposed to be, but I'm looking at the algs file, and it seems like it slipped in by accident. It was supposed to only be used as a device to alter how the stanford compatibility mode worked, and I'm guessing someone mis-understood and added that, and I failed to notice the change.
In fact, the entire rule NUMBER-NUMERIC-QUANTIFIERS looks incorrect ... I'm having trouble understanding what its doing. Argh. Its probably safe to remove it entirely. We should have had far more extensive unit tests much much earlier.
Anyway, ... Hmm _quantity(mile, more) seems wrong to me. More on that in next post.
OK .. I am looking at the google doc for comparatives, and it seems wrong for these sentences, and many others ... It will be difficult if we try to fix three things at once: the unit test, the code, and the google doc, each of which does something different than the other two. Since the unit test will be the final, ultimate authority, I suggest fixing that first. If someone wants to fix the google doc later, that's OK, but, to lessen confusion, we shouldn't use it any more. The unit test cases look almost right. There's a few cases where _num_quantity is being used, this should be changed to just plain _quantity. There's a few cases where _quantity is being used with the comparative, these should be removed entirely. We should not be providing numerical modifiers to "moreness", we should be providing numerical modifiers to the the quantity being compared (miles, number of cookies, etc.) So:
Above looks perfect, to me.
|
@@ -291,7 +292,7 @@ public boolean test_comparatives() | |||
"_quantity(mile, more)\n"+ | |||
"than(he, John)\n" + | |||
"_comparative(mile, run)\n"+ | |||
"_num_quantity(miles, ten)\n" + | |||
"_num_quantity(more, ten)\n" + | |||
"degree(more, comparative)\n"); |
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.
The original code was right, here. ten is modifying miles, not more.
I'm thinking I should not merge this at this time, it seems to introduce as many wrong things, as it fixes ... @ruiting, I assume that you agree with me on these? Or did you expect it to work some other way? |
Hmm I made above changes after having a discussion with @amebel. @amebel you mention that there was a discussion about when to use num_quantity() relationships? |
Um, OK, so the proposal is to use _num_quantity instead of _quantity when the modifier is explicitly numeric? In my opinion, a more elegant solution would be to leave this as _quantity, and add a property numeric-FLAG(10, T) . This seems more elegant because the numeric-ness is associated with the single adjective "10", and not the relation: there is nothing about _num_quantity that makes it or forces it to be actually numeric; you could have "many" or "few" be the modifiers: for example: "she runs a few miles" would have a valid parse _num_quantity(miles, few) because there is nothing explicit in the relationship that is making it actually be numeric. It seems strange to me to say that "few" is "numeric", since it isn't. |
@linas Thanks. This sounds good to me.. |
@linas I have added numeric-FLAG() and changed the NUMBER-NUMERIC-QUANTIFIERS rule and made other relevant fixes based on the discussion. Please review and merge. Thanks |
@@ -1830,18 +1840,21 @@ QUERY_INDEFINATE_DETERMINER2 | |||
#TemplateActionAlg | |||
QUANTITY_EXCEPTION | |||
<LAB> = \D\.*|\DD\.* | |||
<F_L str> = more|More|'s|fewer|Fewer|'|’s|’ | |||
<F_L str> = more|More|less|Less|'s|fewer|Fewer|'|’s|’ |
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.
Do you actually see upper-case More Less ? that seems like a bug to me.
the ' 's is normally a posessive, i'm surprised that its listed here ...
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 mean, a bug in link-grammar, not relex. link-grammar should be downcasing most words....
_quantity() relationship modifications
No description provided.