Coref Components #17
Coref Components #17
Conversation
These come from the feature/coref branch.
These are copied from the feature/coref branch
Tests work now
|
There may still be a few rough edges in the code, but I believe at this point it's ready for review, and strange types / lack of clarity should be mostly resolved. |
The EXXX assignment will have to be made later.
|
You can also test a fully working component with the project here. explosion/projects#101 |
These should be provided by the default config.
…al into refactor/coref-perf
Co-authored-by: kadarakos <kadar.akos@gmail.com>
`Coref`: Optimize `create_gold_scores`
svlandeg
dismissed
their stale review
Decided to continue looking into the performance profiling first
Coerce scalar tensors to native Python integers to avoid comparison overhead.
…mental into refactor/coref-perf
`Coref`: Optimize `SpanResolver.set_annotations`
`Coref`: Vectorize `get_predicted_antecedents`
|
OK, now that performance profiling is done for the time being I think this is ready to merge? |
I believe this fails on Windows due to type issues.
The array created by full defaults to int32, while the input is int64. Linux happily converts this without issue, but Windows throws an error. Making sure the dtype matches resolves the issue.
|
OK, there was a small issue with the optimizations on Windows, but now the tests have cleared (including project tests on Windows) and I was able to train a whole pipeline without issue, so this should be ready to merge, pending any further review. |
No idea if this'll work, but want to check.
|
I am going to see what happens if we add Windows to the CI, though if there are issues with other components that could be out of scope for this PR. |
|
Well, it looks like the tests for this repo on Windows just work, so I guess we can add that too if we want. |
There was a problem hiding this comment.
There's still a few TODO items we're aware of, but it would be great to get this merged into spacy-experimental and start getting feedback from our user base on this functionality!
Not merging yet as we'll need to coordinate across repo's.
Nice work, Paul, Ákos & Madeesh!
This is a continuation of explosion/spaCy#7264, since we decided to add the coref components here first. It's still a work in progress.