Fix handling of small docs in coref #28
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Docs with one or zero tokens fail in the coref component. This doesn't have a fix yet, just a failing test. (There is also a test for the span resolver, which does not fail.)
It might be better to include this optionally? On the other hand, since it should just be ignored in training, having it always there is more thorough.
There can be no coref prediction for docs with one token (or no tokens). Attempting to treat docs like that normally causes a mess with size inference, so instead they're skipped. In training, this just involves skipping the docs in the update step. This is simple due to the fake batching structure, since the batch doesn't have to be maintained. In inference, this just involves short-circuiting to an empty prediction.
The retokenization test is hard-coded to the the training example because it manually merges some tokens, to make sure that the prediction and merge line up. It would probably be better to separate out the training data from the general example here, but for now narrowing the training data works.
|
This should be fixed now, but the tests are failing - seems to be in a non-coref test, so either a dependency was update or it's a fluke. Separately, while this works, I think the tests need some cleanup, so I'll take care of that before bringing this out of draft. |
|
It looks like the test failure was transient - it was in coref tests, but only in a part unaffected by this PR, and it went away on a re-run. On looking at the test code, it seems like factoring data out would only make things messier at this point, so this should be ready as-is if there are no lurking issues. |
kadarakos
approved these changes
Nov 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Docs with one or zero tokens fail in the coref component. This doesn't have a fix yet, just a failing test. (There is also a test for the span resolver, which does not fail.)