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

pydrake: Should ensure that torch>=1.5.0 does not create an issue, relax warning #13707

Closed
EricCousineau-TRI opened this issue Jul 18, 2020 · 3 comments · Fixed by #13708
Closed

Comments

@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented Jul 18, 2020

In #12073, the solution (#12108) was to print a warning that points to this issue:
pytorch/pytorch#3059 (comment)

That pytorch issue was closed in the following commit, which is part of torch>=1.5.0:
pytorch/pytorch@ddff4ef

However, if I now test in a virtualenv with torch==1.5.1, our warning still triggers:

(
set -eux

drake_base=drake-20200718-bionic.tar.gz
drake_file=~/Downloads/${drake_base}
if [[ ! -f ${drake_file} ]]; then
  wget https://drake-packages.csail.mit.edu/drake/nightly/${drake_base} -O ${drake_file}
fi

cd $(mktemp -d)

python3 -m virtualenv -p python3 . --system-site-packages
set +eux
source ./bin/activate
set -eux

tar -xzf ~/Downloads/drake-20200718-bionic.tar.gz --strip-components=1
pip install torch==1.5.1

python3 -c 'import torch; from pydrake.examples.acrobot import AcrobotPlant; AcrobotPlant()'
)

I'm bringing this up b/c I'm looking into #13571 and isort, and in Anzu, our import logic gets a little messy if we leave the condition that pydrake should be imported before pytorch. (Also, false positives here are annoying.)

@EricCousineau-TRI
Copy link
Contributor Author

Old repro is here:
#12073 (comment)

@EricCousineau-TRI
Copy link
Contributor Author

Updated repro: branch | commit (still for torch==1.0.0)
Command:

cd drake/tmp
./repro.sh

@EricCousineau-TRI
Copy link
Contributor Author

EricCousineau-TRI commented Jul 18, 2020

Up to commit 9732ec0, found that torch=={1.0.0, 1.1.0, 1.2.0, 1.3.0} all failed the ./repro.sh recipe.
However, torch=={1.4.0, 1.5.1} did not fail it.

Will see if this pattern aligns with Sean's example w/ AcrobotPlant() - but only testing torch=={1.3.0, 1.4.0, 1.5.1}

EDIT: Yup, pattern aligned. Failed on 1.3.0, but not other versions. Makes sense why 1.5.1 works, but dunno why 1.4.0 works, but 🤷

EDIT 2: Huh... when looking at pytorchs source, I really dunno why 1.4.0 works.
This command doesn't show any significant diffs, so they must've changed how they packaged things:

cd pytorch
git diff v1.3.0 v1.4.0 -- torch/__init__.py

EDIT 3: Testing versions:

  • 1.5.1: Commit 5236cb4, does not warn, does not segfault (good)
  • 1.4.0: Commit f77f4b1, warns, but does not segfault
  • 1.3.0: Commit 19c8194, warns, and segfaults

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant