Tensor init refactor #175
Closed
Tensor init refactor #175
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
Not sure what the reason for this attribute is? I have removed it for now.
Instead of our code checking supported data types, let numpy check if the value can be converted to an ndarray. We only care if it is an ndarray or not anyway.
Can't have GPUBuffer without GPU=True
Reduce a line as data will always have a dtype at this point.
geohot
reviewed
Dec 10, 2020
| data = np.array(data, dtype=np.float32) | ||
| elif GPU and isinstance(data, GPUBuffer): | ||
| self.gpu = True | ||
| self.gpu = False |
There was a problem hiding this comment.
default_gpu is never set True unless the user at runtime sets Tensor.default_gpu = True. Perhaps this was supposed to read from the system environment(export GPU=true)?
But that being said, setting default_gpu=True fails the tests anyway. This is due to the way .cuda_ handles conversions.
For this reason, I removed it and if support needs to be added again, we should refactor .cuda_ and .cuda().
There was a problem hiding this comment.
I also believe that it would be better in the future to add default GPU support in the kwargs of the __init__.
I.e.
def __init__(self, data, gpu=GPU, requires_grad=True):Or something like that.
geohot
reviewed
Dec 10, 2020
|
Did parts of this, removed default. Want to rebase and I'll merge? |
|
Not required. See #196 for even better init. |
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.
No description provided.