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.

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

Updating torch.range to torch.arange #378

Merged
merged 6 commits into from Aug 18, 2022
Merged

Updating torch.range to torch.arange #378

merged 6 commits into from Aug 18, 2022

Conversation

jorshi
Copy link
Collaborator

@jorshi jorshi commented Aug 16, 2022

torch.range is now deprecated and should be replaced with torch.arange, which is consistent with pythons built-in range. torch.range also was producing errors with high-valued ranges, see #377

Merge #380 into this first.

@jorshi jorshi requested a review from turian August 16, 2022 06:17
@turian
Copy link
Collaborator

turian commented Aug 16, 2022

Yeah but what about older versions of pytorch? Do we break them? We should run a full jenkins

@turian
Copy link
Collaborator

turian commented Aug 16, 2022

Since when is it deprecated? Do you have a link?

@jorshi
Copy link
Collaborator Author

jorshi commented Aug 16, 2022

@turian torch.range has been deprecated in favour of torch.arange since torch v0.1 https://github.com/pytorch/pytorch/releases/tag/v0.1.12 -- so we should be good for backward compatibility :)

@turian
Copy link
Collaborator

turian commented Aug 17, 2022

Okay. Why are tests still breaking?

@jorshi
Copy link
Collaborator Author

jorshi commented Aug 17, 2022

Tests were breaking because the deprecated function call Trainer.test(test_dataloader=) was removed from Lightning (formally PTL) in version 1.6, see Lightning-AI/pytorch-lightning#10325. test_dataloader was deprecated in v1.4.0, see Lightning-AI/pytorch-lightning#7431. Replacing Trainer.test(test_dataloader=) with Trainer.test(dataloader=) will work for ptl>=1.4

@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2022

This pull request introduces 1 alert when merging 395c6ab into 23d6411 - view on LGTM.com

new alerts:

  • 1 for Unused import

@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #378 (4bf6187) into main (a2fdb48) will decrease coverage by 0.38%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #378      +/-   ##
==========================================
- Coverage   97.06%   96.68%   -0.39%     
==========================================
  Files          10        9       -1     
  Lines         887      694     -193     
==========================================
- Hits          861      671     -190     
+ Misses         26       23       -3     
Flag Coverage Δ
pytest 96.68% <100.00%> (ø)
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
torchsynth/profile.py 100.00% <100.00%> (ø)
torchsynth/synth.py 94.00% <100.00%> (-0.67%) ⬇️
torchsynth/module.py 96.44% <0.00%> (-2.67%) ⬇️
examples/examples.py

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@turian turian merged commit a51a36c into main Aug 18, 2022
@turian turian deleted the torch-range branch August 18, 2022 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants