Pattern matcher fixes

[GregDubbin]

#1503

Description

This PR includes fixes and improvements to the pattern matching behavior of spacy.matcher.Matcher. Specifically, the '*' and '+' operators are not greedy and are inconsistent with the output of standard regular expression libraries.

The PR adds a ADVANCE_PLUS action that behaves as if both ADVANCE_ZERO and REPEAT were the action. This allows greedy matching to be implemented without backtracking. Due to the restrictions of the token patterns (i.e. one operator per token), the partial match list can be pruned to never be larger than twice the number of specs in a pattern.

Types of change

1. Fixed bug in matching state logic preventing '*' from ever being greedy
2. Expanded functionality to allow multiple variable length quantifiers without backtracking

Checklist

• [ x] I have submitted the spaCy Contributor Agreement.
• [ x] I ran the tests, and all new and existing tests passed.
• [ x] My changes don't require a change to the documentation, or if they do, I've added all required information.

[explosion-bot]

Hi @GregDubbin, thanks for your pull request! 👍 It looks like you haven't filled in the spaCy Contributor Agreement (SCA) yet. The agrement ensures that we can use your contribution across the project. Once you've filled in the template, put it in the .github/contributors directory and add it to this pull request. If your pull request targets a branch that's not master, for example develop, make sure to submit the Contributor Agreement to the master branch. Thanks a lot!

If you've already included the Contributor Agreement in your pull request above, you can ignore this message.

[honnibal]

Hmm, it compiles fine for me but the CI servers are giving:

/usr/include/c++/4.8/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support is currently experimental, and must be enabled with the -std=c++11 or -std=gnu++11 compiler options.

[honnibal]

I think the compile error is coming from the unordered map. We could replace this with the PreshMap hash table I use elsewhere in spaCy, but I think it's a bit inconvenient, since it's nice to map directly to a struct, as you do here. PreshMap only supports 64-bit keys and values. I'll try adding the compiler flag they suggest.

I wish I could make the overall matching code simpler and better. There are a few block that are fairly repetitive, but it's hard to refactor it into functions without actually making things worse.

Thanks a lot for your help here. This has been broken for a long time, and it's really not an easy bit of code to work with. I think your solution looks very good.

[GregDubbin]

I believe you are correct. An alternative is to include a pointer to a MatchEntryC in TokenPatternC data structure, initializing it so that every TokenPatternC in a pattern points to the same MatchEntryC. This should save some memory and time as we wouldn't have to move to the end of the pattern to check the last match for the pattern.

[GregDubbin]

I believe that the test examples in test_issue1450 are incorrect. Specifically, ('a b b c', 0, 2) should be ('a b b c', 0, 3) and ('a b b', 0, 2) should be ('a b b', 0, 3). I have changed the code accordingly, let me know if you disagree.

[honnibal]

Amazing work! And yes I think you're right about that test: I think it matched the incorrect performance.

Okay I think this is good to merge --- a major improvement!