Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Parametrizing RNN tests #441

Merged
merged 2 commits into from
Apr 16, 2020
Merged

Parametrizing RNN tests #441

merged 2 commits into from
Apr 16, 2020

Conversation

cjermain
Copy link
Contributor

This PR parametrizes the RNN tests to run over the RNN classes (SimpleRNN, GRU, and LSTM). This simplifies the existing test functions, and provides the ability to filter on a specific class of interest. For example, to run all of the GRU tests, you can filter on GRU:

$ pytest tests -k GRU -vv
===================================== test session starts =====================================
...
collected 163 items / 152 deselected / 1 skipped / 10 selected                                

tests/test_layers.py::test_GRU PASSED                                                   [  9%]
tests/test_layers.py::test_Bidirectional[GRU-True] PASSED                               [ 18%]
tests/test_layers.py::test_Bidirectional[GRU-False] PASSED                              [ 27%]
tests/test_layers.py::test_Bidirectional_with_bias[GRU] PASSED                          [ 36%]
tests/test_layers.py::test_Bidirectional_with_initial_states[GRU] PASSED                [ 45%]
tests/test_layers.py::test_Bidirectional_seqlen_none[GRU] PASSED                        [ 54%]
tests/test_layers.py::test_rnn_state_passing[GRU] SKIPPED                               [ 63%]
tests/test_layers.py::test_masking[GRU] PASSED                                          [ 72%]
tests/test_layers.py::test_masking_bias[GRU] PASSED                                     [ 81%]
tests/test_layers.py::test_masking_bias_bidirectional[GRU] PASSED                       [ 90%]
tests/test_layers.py::test_masking_value[GRU] PASSED                                    [100%]

====================================== warnings summary =======================================
...
================== 10 passed, 2 skipped, 152 deselected, 1 warning in 7.68s ===================

jiafatom
jiafatom previously requested changes Apr 16, 2020
View reviewed changes
Copy link
Collaborator

@jiafatom jiafatom left a comment •
edited
Loading

Choose a reason for hiding this comment

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

If I want to run a single test in test_layers.py, what is the best way to do in the current pytest setting? I want to run a single test today in pycharm, for example test_Bidirectional, but it looks not convenient than before :( (Previously I just click on the run button and it can work). Thanks a lot!

@jiafatom jiafatom dismissed their stale review April 16, 2020 05:19

synced offline

@cjermain
Copy link
Contributor Author

If I want to run a single test in test_layers.py, what is the best way to do in the current pytest setting? I want to run a single test today in pycharm, for example test_Bidirectional, but it looks not convenient than before :( (Previously I just click on the run button and it can work). Thanks a lot!

While I am not familiar with the PyCharm integrations, it seems that there is documentation on using pytest features in the following:
https://www.jetbrains.com/help/pycharm/pytest.html

I typically run a single test by filtering for that test name using the -k flag:

pytest tests -k test_Bidirectional

It will match anything with the string "test_Bidirectional". There are a number of additional features for getting more specific. https://docs.pytest.org/en/latest/usage.html#specifying-tests-selecting-tests

@wenbingl
Copy link
Member

wenbingl commented Apr 16, 2020 •
edited
Loading

@cjermain , any idea of this test_Bidirectional_with_bias[LSTM] failure on Python35-tf1.11.0? Is it related to the PR or is it a random failure.

@wenbingl
Copy link
Member

@cjermain , any idea of this test_Bidirectional_with_bias[LSTM] failure on Python35-tf1.11.0? Is it related to the PR or is it a random failure.

I rerun the CI check, the issue is gone. I guess the test needs backend.clear_session to make the test robust.

@wenbingl wenbingl merged commit c6e2cd7 into onnx:master Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wenbingl wenbingl wenbingl approved these changes

@jiafatom jiafatom jiafatom left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cjermain @wenbingl @jiafatom