run all tests #4733
run all tests #4733
Conversation
gwenzek
force-pushed
the
run_tests
branch
2 times, most recently
from
fe5e3b1
to
db1e645
Compare
gwenzek
requested review from
kahne,
sravyapopuri388 and
jmp84
as code owners
|
Note that using this PR I managed to reproduce #4429 in CI: https://github.com/facebookresearch/fairseq/actions/runs/3093135329/jobs/5005173667 It only happens when using |
This should also help catch some import bugs, since sphinx inspect a lot of code
pyproject.toml
Outdated
| "wheel", | ||
| "cython", | ||
| "numpy>=1.23.3", | ||
| "torch>=1.12", |
There was a problem hiding this comment.
According to our documentation we are supporting all PyTorch versions newer than 1.5. Considering v1.5 is very archaic, we can revise that requirement, but I think it would be safer to specify >=1.10 or >=1.11, which are still used pretty widely.
There was a problem hiding this comment.
good point. I've changed it to 1.10, and updated the readme accordingly. I also changed py3.6 to py3.8 since we aren't anymore running 3.6 in CI it will likely break silently at some point anyway.
|
I've modified the CI to run on a more recent pytorch release: 1.10. |
run all tests (facebookresearch#4733)
|
@gwenzek After this commit was merged, I encountered an error when doing |
|
Hi, could you report the pip version you're using ? and torch ? Also for me the error message is pretty clear, and not related to my commit. |
|
@gwenzek Hi, my torch/pip version is: The message is strange because my torch is indeed compiled with CUDA 11.6 ( |
|
@freewym hi,I have encountered the same problem. Have you solved it？ |
|
@sevensix617 I haven't. Are you using the same version of PyTorch? |
|
Hi, I'm sorry, but I have a hard time reproducing. Could someone share a detailed script from a new conda env to the error ? |
|
@freewym Yes, I am the same as your torch and cuda versions, and the errors are the same. |
I ran the build when building a docker: after adding "--no-build-isolation" to pip install, pip install seems to work now. However, when running the following I resolved it by adding Do you have any idea of why |
|
Basically the change I made says that we need torch installed to install fairseq, which was already the case. This allows to use fairseq has a dependency in another project. This is important to avoid needing everyone to commit their research experiment inside fairseq. The problem with my approach is that when setuptools sees that "torch" is required at build time, it sometime decides to download a new version of pytorch instead of using the one on the current conda env. They call that "build isolation" and they think it's a feature: https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/#build-isolation But currently there is no way of saying "I want the same version of pytorch during build and install". See https://discuss.python.org/t/support-for-build-and-run-time-dependencies/1513/80 for lengthy discussion about this. So the best workaround I found so far is to disable build isolation so that you're sure to reuse the same pytorch. In anycase before my PR: Now: And the |
|
@gwenzek I see. Thanks for your detailed explanations! |
What does this PR do?
Currently only tests files with a
if __name__ == '__main__'will be run.We want to run all tests in tests/
Example: https://github.com/facebookresearch/fairseq/blob/279796224f7c1e89d1c431a8a3d223b471b36bf9/tests/test_binarizer.py#L121-L123
Since Fairseq installation was broken #4726 but the CI was green I also realize the CI was relying too much on
pip install --editableand was making it hard to reproduce some of the bugs users would see.I propose to run CI with a true installation.
This also lead me to find other bugs in setup.py
Fairseq needs pytorch to correctly installed it's binaries, so pytorch should be installed before we look at setup.py.
So I promoted pytorch to a build requirements.
I also bumped the required version of numpy and pytorch so that we fetch version compatible with python 3.8.