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

Add destructors for states in TransitionSystem #4686

Merged

Conversation

adrianeboyd
Copy link
Contributor

@adrianeboyd adrianeboyd commented Nov 20, 2019

Description

Add destructors for states in TransitionSystem to fix memory in beam_parse().

Needs to be coordinated with the parallel modifications to thinc, see explosion/thinc#123.

Fixes #4432.

Types of change

Bugfix.

Checklist

  • I have submitted the spaCy Contributor Agreement.
  • 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.

@adrianeboyd
Copy link
Contributor Author

The tests will fail with the current version of thinc due to API differences.

@adrianeboyd adrianeboyd added bug Bugs and behaviour differing from documentation feat / ner Feature: Named Entity Recognizer feat / parser Feature: Dependency Parser perf / memory Performance: memory use labels Nov 21, 2019
@honnibal
Copy link
Member

Thanks! Just merged the patch to Thinc. When we're ready, we'll put up a dev release of the next version, and then we can require that dev release on this branch. If it goes green we can then release the version of Thinc, and update the requirement to the non-dev release.

@honnibal
Copy link
Member

honnibal commented Dec 9, 2019

The dev version of Thinc should be up now. Closing and reopening to restart checks.

@honnibal honnibal closed this Dec 9, 2019
@honnibal honnibal reopened this Dec 9, 2019
@honnibal honnibal merged commit 38e1bc1 into explosion:master Dec 10, 2019
@svlandeg svlandeg mentioned this pull request Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and behaviour differing from documentation feat / ner Feature: Named Entity Recognizer feat / parser Feature: Dependency Parser perf / memory Performance: memory use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak with beam_parse method
2 participants