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

Tregex: infinite loop for ?<... #1375

Closed
tanloong opened this issue Jul 16, 2023 · 6 comments
Closed

Tregex: infinite loop for ?<... #1375

tanloong opened this issue Jul 16, 2023 · 6 comments

Comments

@tanloong
Copy link
Contributor

Specifying the MULTI_RELATION as optional seems to cause an infinite loop of printing matches. I am using Tregex 4.2.0 downloaded from https://nlp.stanford.edu/software/tregex.shtml#Download and here is the command I used:

echo "(A (B 1) (C 2))" | tregex.sh "A ?<... { B ; C }" -filter

tregex_optional-multi-relation

P.S.

I noticed that the testSubtreePattern of TregexTest.java includes the <... and !<... but does not cover ?<... . So I gave it a try and encountered this issue.

@AngledLuffa
Copy link
Contributor

Thanks, love it. A bug in an obscure syntax I added because I was tired of writing out the individual pieces by hand, using a variant of the feature probably no one will ever use. I'll try to figure out what's going on.

Interesting to note btw is that when the pattern is turned back into a tregex, it does not use the <... syntax. Basically, the <... gets expanded into the component pieces it represents.

I suspect the optional syntax in general is wrong somehow... there shouldn't be anything specific to the <..., considering your expression gets turned into a nested CoordinationPattern with two < and one !<

@AngledLuffa
Copy link
Contributor

Haven't fully generalized this yet, but this also goes infinite:

echo "(A (B 1) (C 2))" | java edu.stanford.nlp.trees.tregex.TregexPattern "A ?(< B < C)" -filter

This does not:

echo "(zzz (A (B 1) (C 2)))" | java edu.stanford.nlp.trees.tregex.TregexPattern "A ?(< B)" -filter

So basically any optional CoordinationPattern is buggy?

@AngledLuffa
Copy link
Contributor

... for reference, CoordinationPattern is how Tregex implements an and or an or internally

further bugginess in it is that the pattern

A ?(< B < C)

gets output without the optional:

(A < B < C )

this is probably easier to fix

@AngledLuffa
Copy link
Contributor

well, this fixed the output of the CoordinationPattern, at least

a9965b2

@AngledLuffa
Copy link
Contributor

Alright, figured out what's going on - basically, the CoordinationPattern in question wasn't checking if it had already returned once for an optional pattern, instead just always returned true for an optional. The result was it would just always accept new empty matches every time it checked if the tree had another match...

under the hood, in case it wasn't clear from the previous comments, <... is just a macro for match the named nodes, then check that there isn't a subsequent node at the end of the list, and do all of that in a conjunction CoordinationPattern

AngledLuffa added a commit that referenced this issue Jul 17, 2023
…s would infinite loop, as the matcher would accept a failed match even after a previous success (meaning it would always be willing to accept the same failed match again). #1375
@tanloong
Copy link
Contributor Author

tanloong commented Jul 17, 2023

Yes. The A ?<... { B ; C } gets expanded to A <1 B <2 C !<3 __.

Thanks for all the efforts and the concise explanation!

AngledLuffa added a commit that referenced this issue Sep 6, 2023
…s would infinite loop, as the matcher would accept a failed match even after a previous success (meaning it would always be willing to accept the same failed match again). #1375
AngledLuffa added a commit that referenced this issue Sep 6, 2023
…junctions would infinite loop, as the matcher would accept a failed match even after a previous success (meaning it would always be willing to accept the same failed match again). #1375"

This reverts commit af5d6d7.
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