Performance improvements #82
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
added 11 commits
April 1, 2019 21:44
|
Tests are passing, this should be ready to review now. |
jzuhone
reviewed
Apr 3, 2019
jzuhone
approved these changes
Apr 3, 2019
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.
This includes fixes for several performance issues I found integrating unyt with yt.
Big changes are refactoring of parsing into a new
unyt._parsingprivate module, which allows me to refactor theUnitSystemclass to parse unit names and store unit expressions internally, a big performance win since we can avoid repeatedly and expensively parsing unit strings whenever we go through the unit system machinery.This also includes a small behavior change in the cgs/mks conversion rules to make them slightly more relaxed. This leads to a big performance wins for all unit conversions so I definitely think it's worth it, see the test changes for the extent of the changes.
Last there are a few other more minor or isolated changes that were all clear-cut performance wins and didn't lead to any test changes.
So long as @jzuhone approves of the test changes I'm going to cut another release based on this PR so I can update the yt tests and hopefully substantially improve the performance over there.