contraction_list: only keep remaining for last 20 contractions #149
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
dgasmith
reviewed
Jul 16, 2020
There was a problem hiding this comment.
You are hitting #147 in the CI as well, for that test can you add a check for (ValueError, TypeError) with a comment like, # changed in Numpy 1.19, see https://github.com/numpy/numpy/commit/35b0a051c19265f5643f6011ee11e31d30c8bc4c?
It's a harmless change.
| @@ -51,16 +51,22 @@ def __repr__(self): | |||
| " Complete contraction: {}\n".format(self.eq), " Naive scaling: {}\n".format(len(self.indices)), | |||
| " Optimized scaling: {}\n".format(max(self.scale_list)), " Naive FLOP count: {:.3e}\n".format( | |||
| self.naive_cost), " Optimized FLOP count: {:.3e}\n".format(self.opt_cost), | |||
| " Theoretical speedup: {:3.3f}\n".format(self.speedup), | |||
| " Theoretical speedup: {:.3e}\n".format(self.speedup), | |||
There was a problem hiding this comment.
+1 the values were getting out of hand.
opt_einsum/contract.py
Outdated
| @@ -303,7 +309,12 @@ def contract_path(*operands, **kwargs): | |||
|
|
|||
| einsum_str = ",".join(tmp_inputs) + "->" + idx_result | |||
|
|
|||
| contraction = (contract_inds, idx_removed, einsum_str, input_list[:], do_blas) | |||
| if len(input_list) <= 10: | |||
There was a problem hiding this comment.
Any harm in extending this to 15-20?
There was a problem hiding this comment.
None memory wise! 10 was just a rough judgement call on neatness, but 20 feels fine as well.
|
OK, raised number of 'remaining terms' kept to 20 and should have fixed #147 now as well. |
jcmgray
changed the title
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.
Description
Resolves #148, by only keeping the last 10 'remaining terms' lists in the
contraction_list, which is generally the point it starts to look a bit messy in thePathInfoobject. This could be tweaked / made customizable.I've also
[...]might also be clearer.Status