Albert Zeyer

Results 947 comments of Albert Zeyer

I guess @JackTemaki and/or @patrick-wilken should otherwise review this.

Of course, in the case of the `tf.data` pipeline, `batch_dim` would not be feeded (be a `tf.placeholder`) but also come directly from the pipeline.

For anyone working on this: A problem why this issue was not noticed earlier is that we also lack a test case for this. So we should first also make...

I noticed, we actually do have test cases already for this, for example `test_engine_train_new_dataset_pipeline`. So why was this unnoticed? Maybe because this specific test case never makes use of the...

Thanks for the PR. There are some issues with it. As a starting point for contributing, see here: https://github.com/rwth-i6/returnn/blob/master/CONTRIBUTING.md * [x] The syntax style does not follow PEP8 and other...

Can you see the output of the failing tests? For example: ``` returnn/tf/util/data.py:21: WEAK WARNING Pep8CodeStyle: E303: too many blank lines (3) returnn/tf/util/data.py:990: WEAK WARNING Pep8CodeStyle: E501: line too long...

Btw, I just introduced a new behavior version, so you should rebase here.

See my updated comment above. As an example on the test cases: ```python def test_Data_get_common_data_extra_static_spatial(): d1 = Data(name='t', shape=(None, 32, 128), dtype='float32', auto_create_placeholders=True) d2 = Data(name='r', shape=(None, 32, 128), dtype='float32',...

You don't need to run all the tests. You can also just check them here in the issue, or otherwise only run `test_TFUtil.py`, `test_TFEngine.py`, `test_TFNetworkLayer.py`, `test_TFNetworkRecLayer.py`.

> It seems like we have to go for that implementation Why? As I said, I really would try to avoid this. Did you try my other suggestion with removing...