Skip to content

Commit

Permalink
MergeDimsLayer, only allow keep_order=True
Browse files Browse the repository at this point in the history
Fix #654.
  • Loading branch information
albertz committed Nov 27, 2021
1 parent 95844ea commit 6dc73f0
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
10 changes: 10 additions & 0 deletions docs/configuration_reference/behavior_version.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ and not listing legacy/deprecated parameters.
Version History
---------------

Behavior version 6 (2021-11-27)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

:class:`MergeDimsLayer` uses ``keep_order=True`` and does not allow ``keep_order=False``.
There never should be a reason to use ``keep_order=False`` anyway.
If you have that, just remove it.
If that causes any problems, there is probably some other issue in your config.

See issue `#654 <https://github.com/rwth-i6/returnn/issues/654>`__.

Behavior version 5 (2021-11-26)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
18 changes: 13 additions & 5 deletions returnn/tf/layers/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2838,20 +2838,26 @@ class MergeDimsLayer(_ConcatInputLayer):
"""
layer_class = "merge_dims"

def __init__(self, axes, keep_order=False, n_out=None, **kwargs):
def __init__(self, axes, keep_order=NotSpecified, n_out=None, **kwargs):
"""
:param str|list[str]|list[int] axes: see Data.get_axes_from_description(), e.g. "except_time"
:param bool keep_order: By default (for historical reasons), the axes are sorted, and then merged.
:param bool|NotSpecified keep_order: The old default was: the axes are sorted, and then merged.
Thus, the order of incoming axes will influence the result.
E.g. inputs [B,S,F] and [B,F,S], with ``axes=["S","F"]``, will get different results,
although the output shape is [B,S*F] in both cases.
This is bad: In general, other layers in RETURNN might reorder the axes for various reasons,
and all layers should behave in the same way, no matter the order.
It is recommended to set ``keep_order=True``, such that the order defined in ``axes`` defines the behavior,
and not the incoming axis order.
Since behavior version 6, this is already the case.
:param int|None n_out:
"""
from returnn.util import BehaviorVersion
super(MergeDimsLayer, self).__init__(**kwargs)
if keep_order is NotSpecified:
keep_order = True if BehaviorVersion.get() >= 6 else False
BehaviorVersion.require(
condition=keep_order, message="MergeDimsLayer, only keep_order=True is allowed", version=6)
if keep_order:
assert isinstance(axes, (tuple, list)), "%s: unique axes %r required" % (self, axes)
axes_ = []
Expand Down Expand Up @@ -2971,18 +2977,20 @@ def _set_output_sizes(self, merge_axes):
target_tag.dyn_size_ext = out_size

@classmethod
def get_out_data_from_opts(cls, name, axes, keep_order=False,
def get_out_data_from_opts(cls, name, axes, keep_order=NotSpecified,
sources=(), n_out=NotSpecified, out_type=None, **kwargs):
"""
:param str name:
:param str|list[str] axes:
:param bool keep_order:
:param bool|NotSpecified keep_order:
:param list[LayerBase] sources:
:param int|None|NotSpecified n_out:
:param None|dict[str] out_type:
:rtype: Data
"""
from ..util.data import DimensionTag
from returnn.util import BehaviorVersion
if keep_order is NotSpecified:
keep_order = True if BehaviorVersion.get() >= 6 else False
assert not out_type, "currently ignored"
input_data = get_concat_sources_data_template(sources)
data = input_data.copy(name="%s_output" % name)
Expand Down
2 changes: 1 addition & 1 deletion returnn/util/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class BehaviorVersion:
The version will be set after the config is defined at __main__.init_config() or Engine.__init__()
"""

_latest_behavior_version = 5
_latest_behavior_version = 6
_behavior_version = None # type: typing.Optional[int]

@classmethod
Expand Down

0 comments on commit 6dc73f0

Please sign in to comment.