rwth-i6 / pytorch-to-returnn

Make PyTorch code runnable within RETURNN
3 stars 6 forks source link

implement bidirectional LSTM #60

Closed vieting closed 2 years ago

albertz commented 2 years ago

For reference, the error:

...
  File "/home/runner/work/pytorch-to-returnn/pytorch-to-returnn/tests/test_layers.py", line 969, in test_blstm
...
  File "/home/runner/work/pytorch-to-returnn/pytorch-to-returnn/pytorch_to_returnn/torch/nn/modules/module.py", line 682, in Module.check_call_returnn_outputs_to_prev_torch
    line: numpy.testing.assert_allclose(
            returnn_output_np, torch_out_np,
            err_msg="\n".join(error_msg_info),
            **naming.validate_allclose_kwargs)
    locals:
...
AssertionError: 
Not equal to tolerance rtol=0, atol=0.0005
RETURNN layer: <SubnetworkLayer 'LSTM' out_type=Data{[T|'time:var:extern_data:data'[B],B,F|F'concat_layer0_fwd_layer0_bwd_feature'(26)]}>
Torch module: LSTM(11, 13, bidirectional=True)
  RETURNN output 2/3 min/max: (-0.28403014, 0.2864878)
  Torch output 2/3 min/max: (-0.387018, 0.2690077)
  Biggest mismatch at idx (1, 1, 0), RETURNN 0.23704060912132263 vs Torch -0.203242689371109
Mismatched elements: 38 / 78 (48.7%)
Max absolute difference: 0.4402833
Max relative difference: 213.08286
 x: array([[[ 0.065673,  0.269008,  0.019763,  0.11937 , -0.196358,
          0.155059, -0.08992 , -0.092004,  0.003254,  0.033857,
         -0.136288,  0.031271,  0.028576],...
 y: array([[[ 0.065673,  0.269008,  0.019763,  0.11937 , -0.196358,
          0.155059, -0.08992 , -0.092004,  0.003254,  0.033857,
         -0.136288,  0.031271,  0.028576],...
vieting commented 2 years ago

Why does the test not use the most recent RETURNN version?

RETURNN: 1.20211024.182640+git.8b24699 1.20211024.182640 /home/runner/.local/lib/python3.8/site-packages/returnn/init.py

Commit 8b24699 is the second last, but we need the fix in the last commit.

albertz commented 2 years ago

Why does the test not use the most recent RETURNN version?

RETURNN: 1.20211024.182640+git.8b24699 1.20211024.182640 /home/runner/.local/lib/python3.8/site-packages/returnn/init.py

Commit 8b24699 is the second last, but we need the fix in the last commit.

It depends when it ran the checks. It gets RETURNN from pip, not from the GitHub repo. In the GitHub RETURNN repo, when we push sth to master, it will run through all tests, and when they passed, after they passed, it will do a new pip upload. So after I merged in your RETURNN PR, it usually takes about 10 minutes until it gets to pip (assuming the tests are passing). And pip itself might take a few more minutes for internal caching.

You can just reran the checks. (Or can you? Do you have permissions?) I just did that now. Now we have:

RETURNN: 1.20211026.185242+git.d62b143 1.20211026.185242

Edit And all tests are passing now.

albertz commented 2 years ago

The PR is ready now, or not?

vieting commented 2 years ago

Yes, from my side it's ready.