Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Remove duplicate @with_seed decorators #19336

Merged
merged 4 commits into from
Oct 22, 2020

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Oct 12, 2020

function_scope_seed pytest autuse fixture defined in conftest.py already manages the seeds of every test function.

Relying on the autouse fixture is more robust, as it does not require the contributors to add a explicit @with_seed decorator to contributed test functions.

Please see below code-blocks for examples how the test seed is printed. The first codeblock shows a normal test function, whereas the second shows a unittest.TestCase.

% python3 -m pytest ../tests/python/unittest/test_deferred_compute.py::test_indexing_empty_shape                                                                                                                                     6s ~/src/mxnet-master/build 2020-10/pytest-with-seed-fixture ip-172-31-95-96
============================================================================================================================================== test session starts ===============================================================================================================================================
platform linux -- Python 3.8.5, pytest-6.0.2, py-1.9.0, pluggy-0.13.1
rootdir: /home/ubuntu/src/mxnet-master, configfile: pytest.ini
plugins: timeout-1.4.2, env-0.6.2
timeout: 1200.0s
timeout method: signal
timeout func_only: False
collected 1 item

../tests/python/unittest/test_deferred_compute.py F                                                                                                                                                                                                                                                        [100%]

==================================================================================================================================================== FAILURES ====================================================================================================================================================
___________________________________________________________________________________________________________________________________________ test_indexing_empty_shape ____________________________________________________________________________________________________________________________________________

    def test_indexing_empty_shape():
>       raise RuntimeError
        @mx.util.use_np
E       RuntimeError

../tests/python/unittest/test_deferred_compute.py:581: RuntimeError
--------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup ----------------------------------------------------------------------------------------------------------------------------------------------
Setting module np/mx/python random seeds, use MXNET_MODULE_SEED=845205315 to reproduce.
--------------------------------------------------------------------------------------------------------------------------------------------- Captured log teardown ----------------------------------------------------------------------------------------------------------------------------------------------
WARNING  root:conftest.py:225 Error seen with seeded test, use MXNET_TEST_SEED=215573059 to reproduce
============================================================================================================================================ short test summary info =============================================================================================================================================
FAILED ../tests/python/unittest/test_deferred_compute.py::test_indexing_empty_shape - RuntimeError
=============================================================================================================================================== 1 failed in 0.21s ================================================================================================================================================
% python3 -m pytest ../tests/python/unittest/test_numpy_contrib_gluon_data_vision.py                      6s ~/src/mxnet-master/build 2020-10/pytest-with-seed-fixture ip-172-31-95-96
================================================================================= test session starts =================================================================================
platform linux -- Python 3.8.5, pytest-6.0.2, py-1.9.0, pluggy-0.13.1
rootdir: /home/ubuntu/src/mxnet-master, configfile: pytest.ini
plugins: timeout-1.4.2, env-0.6.2
timeout: 1200.0s
timeout method: signal
timeout func_only: False
collected 3 items

../tests/python/unittest/test_numpy_contrib_gluon_data_vision.py F..                                                                                                                                                                                                                                       [100%]

==================================================================================================================================================== FAILURES ====================================================================================================================================================
_________________________________________________________________________________________________________________________________________ TestImage.test_bbox_augmenters _________________________________________________________________________________________________________________________________________

self = <test_numpy_contrib_gluon_data_vision.TestImage testMethod=test_bbox_augmenters>

    @use_np
    def test_bbox_augmenters(self):
>       raise RuntimeError
E       RuntimeError

../tests/python/unittest/test_numpy_contrib_gluon_data_vision.py:133: RuntimeError
--------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout setup ----------------------------------------------------------------------------------------------------------------------------------------------
Setting module np/mx/python random seeds, use MXNET_MODULE_SEED=541035833 to reproduce.
---------------------------------------------------------------------------------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------------------------------------------------------------------------------
Loaded 16 images
cleanup /tmp/tmpv6padmnc
--------------------------------------------------------------------------------------------------------------------------------------------- Captured log teardown ----------------------------------------------------------------------------------------------------------------------------------------------
WARNING  root:conftest.py:225 Error seen with seeded test, use MXNET_TEST_SEED=1970205131 to reproduce
============================================================================================================================================ short test summary info =============================================================================================================================================
FAILED ../tests/python/unittest/test_numpy_contrib_gluon_data_vision.py::TestImage::test_bbox_augmenters - RuntimeError
========================================================================================================================================== 1 failed, 2 passed in 4.31s ===========================================================================================================================================

@leezu leezu requested review from DickJC123 and szha October 12, 2020 17:50
@mxnet-bot
Copy link

Hey @leezu , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [sanity, miscellaneous, windows-gpu, windows-cpu, unix-cpu, centos-gpu, centos-cpu, edge, website, unix-gpu, clang]


Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

function_scope_seed pytest autuse fixture defined in conftest.py already manages
the seeds of every test function.
@leezu leezu force-pushed the 2020-10/pytest-with-seed-fixture branch from 5b2a883 to f0ca4e0 Compare October 12, 2020 17:51
@DickJC123
Copy link
Contributor

It will be nice to have the functionality of @with_seed without relying on the test creator to add the decorator. I'll take a look at this...

@leezu
Copy link
Contributor Author

leezu commented Oct 12, 2020

It will be nice to have the functionality of @with_seed without relying on the test creator to add the decorator. I'll take a look at this...

@DickJC123 that's the aim of this PR. We already have the function seed functionality in conftest.py below. What do you have in mind to be missing?

https://github.com/apache/incubator-mxnet/blob/5ed72b16d297fa244c1217bbf692cd909bd06d46/conftest.py#L156-L185

@leezu
Copy link
Contributor Author

leezu commented Oct 12, 2020

@DickJC123 please take a look at the fixture implementation:

https://github.com/apache/incubator-mxnet/blob/57802e7242b8822cf4dee250ecb7d38fdb8c5901/conftest.py#L187-L227

I wrote it earlier based on @with_seed for the GluonNLP toolkit. @szha copied it over to MXNet in #18025 and the current PR removes @with_seed in favour of the conftest implementation as both have the same functionality, the fixture is more robust and it may be confusing / error-prone to have both active at the same time.

@leezu
Copy link
Contributor Author

leezu commented Oct 12, 2020

@mxnet-bot run ci [windows-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [windows-gpu]

@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 21, 2020
@leezu
Copy link
Contributor Author

leezu commented Oct 22, 2020

@mxnet-bot run ci [unix-cpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-cpu]

@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test labels Oct 22, 2020
@szha szha merged commit 3f436fb into apache:master Oct 22, 2020
@leezu leezu deleted the 2020-10/pytest-with-seed-fixture branch October 22, 2020 03:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants