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.

Sign up for GitHub

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

Jump to bottom

Casting to long #236

Open
kylebgorman opened this issue Aug 22, 2024 · 3 comments · May be fixed by #264
Open

Casting to long #236

kylebgorman opened this issue Aug 22, 2024 · 3 comments · May be fixed by #264
Labels
question Further information is requested

Comments

@kylebgorman
Copy link
Contributor

Here we cast encoded tensors to the Long dtype, i.e., int64. Why? Surely int32 would be enough. Is there any particular reason for this? Could we save memory by getting rid of the dtype spec?
@kylebgorman kylebgorman added bug Something isn't working enhancement New feature or request question Further information is requested and removed bug Something isn't working enhancement New feature or request labels Aug 22, 2024
@Adamits
Copy link
Collaborator

Adamits commented Oct 31, 2024

Link is broken for me, but I think casting to long is a habit I still have from learning from the pytorch tutorial that came out like 7 years ago with the initial release :D. I am not sure that it is at all necessary.

@kylebgorman
Copy link
Contributor Author

@Adamits if I removed these casts and things work as before would you recommend that PR?

@Adamits
Copy link
Collaborator

Adamits commented Oct 31, 2024

32-bit seems pretty safe to me. My one reason why I am not sure is, I think the tensors are going to be 32-bit on most cuda GPUs anyway, and idk if 64-bit will really be a slow down on CPU. It could be that I dont have a good understanding though, and I don't really see many cases where just making it 32-bit would effect us.

kylebgorman added a commit to kylebgorman/yoyodyne that referenced this issue Oct 31, 2024
@kylebgorman
Removes dtype=torch.long declarations.
4e3588a 
I know of no reason to think these are necessary.

Closes CUNY-CL#236.

Closes CUNY-CL#236.

Closes CUNY-CL#236.

Closes CUNY-CL#236.
@kylebgorman kylebgorman linked a pull request Oct 31, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removes dtype=torch.long declarations kylebgorman/yoyodyne
2 participants
@kylebgorman @Adamits