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

[testing] port test_trainer_distributed to distributed pytest + TestCasePlus enhancements #8107

Merged
merged 12 commits into from
Oct 28, 2020

Conversation

stas00
Copy link
Contributor

@stas00 stas00 commented Oct 27, 2020

This PR:

  • ports test_trainer_distributed to run with pytest - it will skip if gpus < 2.

  • includes various improvements via refactoring now 3 use cases of distributed testing by extending TestCasePlus with a whole set of convenient features:

Feature 1: A set of fully resolved important file and dir path accessors.

In tests often we need to know where things are relative to the current test file, and it's not trivial since the test could be invoked from more than one directory or could reside in different sub-directories. This class solves this problem by sorting out all the basic paths and provides easy accessors to them:

  • pathlib objects (all fully resolved):

    • test_file_path - the current test file path (=__file__)
    • test_file_dir - the directory containing the current test file
    • tests_dir - the directory of the tests test suite
    • examples_dir - the directory of the examples test suite
    • repo_root_dir - the directory of the repository
    • src_dir - the directory of src (i.e. where the transformers sub-dir resides)
  • stringified paths - same as above but these return a string, rather than a pathlib object

    • test_file_path_str
    • test_file_dir_str
    • tests_dir_str
    • examples_dir_str
    • repo_root_dir_str
    • src_dir_str

Feature 2: Get a copy of the os.environ object that sets up PYTHONPATH correctly, depending on the test suite it's invoked from. This is useful for invoking external programs from the test suite - e.g. distributed training.

    def test_whatever(self):
        env = self.get_env()
        # now call the external program, passing ``env`` to it

All these are also documented in testing.rst.

Fixes: #8058

@sgugger, @LysandreJik, @sshleifer

@stas00 stas00 changed the title Distr train test [testing] port test_trainer_distributed to pytest-distributed + TestCasePlus enhancements Oct 27, 2020
@stas00 stas00 changed the title [testing] port test_trainer_distributed to pytest-distributed + TestCasePlus enhancements [testing] port test_trainer_distributed to distributed pytest + TestCasePlus enhancements Oct 27, 2020
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR! It all looks good to me!

docs/source/testing.rst Outdated Show resolved Hide resolved
src/transformers/testing_utils.py Outdated Show resolved Hide resolved
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, love the new properties. Thanks @stas00!

@LysandreJik LysandreJik requested a review from sshleifer October 28, 2020 14:13
@sshleifer
Copy link
Contributor

LGTM! cc @patrickvonplaten for awareness!

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
@sgugger sgugger merged commit 5423f2a into huggingface:master Oct 28, 2020
@stas00 stas00 deleted the distr-train-test branch October 28, 2020 15:52
fabiocapsouza pushed a commit to fabiocapsouza/transformers that referenced this pull request Nov 15, 2020
…asePlus enhancements (huggingface#8107)

* move the helper code into testing_utils

* port test_trainer_distributed to work with pytest

* improve docs

* simplify notes

* doc

* doc

* style

* doc

* further improvements

* torch might not be available

* real fix

* Apply suggestions from code review

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
fabiocapsouza added a commit to fabiocapsouza/transformers that referenced this pull request Nov 15, 2020
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.

[testing] port test_trainer_distributed to run with pytest
4 participants