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

Refactor Coval Scoring code #10875

Merged
merged 14 commits into from
Jun 22, 2022
Merged

Conversation

polm
Copy link
Contributor

@polm polm commented May 30, 2022

Description

This moves the scoring code that was taken from coval into scorer.py, renames some things to be less generic, and adds the license to our list.

Types of change

Refactor

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@polm polm mentioned this pull request May 30, 2022
3 tasks
@polm polm marked this pull request as draft May 30, 2022 10:34
@polm
Copy link
Contributor Author

polm commented May 30, 2022

Note: I'm going to go ahead and refactor the scorers into registered scorers in this PR too.

spacy/ml/models/coref.py Outdated Show resolved Hide resolved
@svlandeg svlandeg added feat / coref Feature: Coreference resolution feat / scorer Feature: Scorer labels May 31, 2022
polm added 2 commits June 3, 2022 14:07
Coref can't be loaded without Torch, so nothing works.
Some of this just involves ignoring types in thorny areas. Two main
issues:

1. Some things have weird types due to indirection/ argskwargs
2. xp2torch return type seems to have changed at some point
spacy/scorer.py Outdated Show resolved Hide resolved
spacy/scorer.py Outdated Show resolved Hide resolved
@polm polm marked this pull request as ready for review June 3, 2022 08:55
@polm
Copy link
Contributor Author

polm commented Jun 3, 2022

I ended up fixing the tests (mostly by skipping when Torch isn't present) and types (mostly by ignoring or removing them) :/ . It is green now at least, and should be ready to merge in the coref branch for further cleanup.

@polm polm requested a review from kadarakos June 3, 2022 09:05
@polm polm mentioned this pull request Jun 6, 2022
3 tasks
spacy/tests/pipeline/test_coref.py Outdated Show resolved Hide resolved
spacy/tests/pipeline/test_coref.py Outdated Show resolved Hide resolved
spacy/scorer.py Outdated Show resolved Hide resolved
spacy/scorer.py Outdated Show resolved Hide resolved
@polm polm merged commit 16894e6 into explosion:feature/coref Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat / coref Feature: Coreference resolution feat / scorer Feature: Scorer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants