Fix the bug of BidirectionalCell

[BeyonderXX]

Description

I did hybridize( ) and pass "valid_length" to the unroll( ) function of BidirectionalCell, then returned AssertionError in line 79. Because symbol.split( ) return a symbol but not a symbol list. Result in the length of inputs is not equal to parameter "length" when call unroll( ) to compute r_outputs and r_states. Symbol support for converting to list through list( ) API, so that use list( ) function insted of _as_list( ).

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage:

• Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
• Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
• Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)

• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

python/mxnet/gluon/rnn/rnn_cell.py

• [line 1055] [line 1069]add _reverse_sequences( ) to calculate "reversed_inputs" and "reversed_r_outputs"
• [line 1055] use list( ) instead of _as_list( )

tests/python/unittest/test_gluon_rnn.py

• [line 603] add test_bidirectional_unroll_valid_length( ) to test the unroll( ) of BidirectionalCell when pass
  valid_length to it.

[larroy]

Im confused, doesn't _as_list return a list?

def _as_list(obj):
    """A utility function that converts the argument to a list if it is not already.

    Parameters
    ----------
    obj : object

    Returns
    -------
    If `obj` is a list or tuple, return it. Otherwise, return `[obj]` as a
    single-element list.

    """
    if isinstance(obj, (list, tuple)):
        return obj
    else:
        return [obj]

I don't understand the fix. Can you paste the error?

[szha]

@larroy _as_list would return a wrong list as it would wrap the single grouped symbol inside, instead of length number of tensors for each time-step, which is what the output should be.

[szha]

@BeyonderXX thanks for the fix. Could you add a test with the proper assertions?

[BeyonderXX]

@BeyonderXX thanks for the fix. Could you add a test with the proper assertions?

I‘m glad to！ I will add a new test case as soon as possible.

[BeyonderXX]

@szha I reproduced the issue in windows, when pass valid_length to unroll( ) of BidirectionalCell, it would raise null pointer error after multiple calls.

[BeyonderXX]

@szha I figured out the issue, because I passed a wrong parameter to unroll( ) which led to out of bounds. I have fixed the problem and rewrited my code in BidirectionalCell. So sorry to disturb you abruptly. Thanks!

[szha]

@BeyonderXX thanks for the fix!