-
Notifications
You must be signed in to change notification settings - Fork 114
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
Small perf improvements #1128
Small perf improvements #1128
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1128 +/- ##
=======================================
Coverage 82.10% 82.10%
=======================================
Files 185 185
Lines 48130 48133 +3
Branches 8669 8669
=======================================
+ Hits 39519 39522 +3
Misses 6444 6444
Partials 2167 2167
|
Torch autograd is on by default :O? |
@@ -123,7 +123,10 @@ def arange(start, stop, step): | |||
def pytorch_funcify_Join(op, **kwargs): | |||
def join(axis, *tensors): | |||
# tensors could also be tuples, and in this case they don't have a ndim | |||
tensors = [torch.tensor(tensor) for tensor in tensors] | |||
tensors = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this comprehension is needed at all when I tested in my PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - no it's not. When I did the autograd experiment, I had to turn it on for the tensors we cared about. We could even close this up |
I'm gonna close this - I don't think there is much being changed given torch autograd is off in our case by default. |
Description
There was a warning about copying a tensor, this change removes that. In addition, until we decide to keep torch autograd going around, I disabled it here to free up a bit of memory during the runtime. This brought back about 80us back from the mean time in the best case.
Related Issue
Checklist
Type of change
📚 Documentation preview 📚: https://pytensor--1128.org.readthedocs.build/en/1128/