-
Notifications
You must be signed in to change notification settings - Fork 856
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
Upgrade pytorch requirement to >=1.2.0 #1565
Conversation
db96e41
to
0a909c4
Compare
Codecov Report
@@ Coverage Diff @@
## master #1565 +/- ##
=======================================
Coverage 97.13% 97.13%
=======================================
Files 56 56
Lines 2091 2091
Branches 342 342
=======================================
Hits 2031 2031
Misses 31 31
Partials 29 29
|
requirements.txt
Outdated
@@ -18,15 +18,14 @@ tqdm>=4.33.0,<5.0.0 | |||
|
|||
# Internal models | |||
scikit-learn>=0.20.2,<0.22.0 | |||
torch>=1.1.0,<1.2.0 | |||
torch>=1.4.0,<2.0.0 |
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.
Is this incompatible with 1.2 and 1.3?
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.
yes, all tests pass with the older versions 1.2.0 and 1.3.0.
relaxing the requirement to torch>=1.2.0,<2.0.0
0a909c4
to
c9a0c85
Compare
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.
just one question, otherwise lgtm!!
@@ -41,9 +41,9 @@ | |||
"pandas>=0.25.0,<0.26.0", | |||
"tqdm>=4.33.0,<5.0.0", | |||
"scikit-learn>=0.20.2,<0.22.0", | |||
"torch>=1.1.0,<1.2.0", | |||
"torch>=1.2.0,<2.0.0", | |||
"tensorboard>=1.14.0,<2.0.0", |
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.
how did we decide on this range?
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 got an import error that looked like this:
ImportError: TensorBoard logging requires TensorBoard with Python summary writer installed. This should be available in 1.14 or above
@vincentschen can we update the Test Plan with details from the multiple version tests? |
Yep! Naively, pinned the version to each minor release and ran all unit/integration tests. |
Description of proposed changes
torch
torch.utils.tensorboard
and removetensorboardX
deplr_scheduler.step()
afteroptimizer.step()
.byte()
with.bool()
Related issue(s)
Fixes #1558
Test plan
torch==1.2.0, 1.3.0, 1.4.0
tox -e {complex,spark}
passes withtorch==1.2.0, 1.3.0, 1.4.0
Checklist
Need help on these? Just ask!
tox -e complex
and/ortox -e spark
if appropriate.