Skip to content

Simpler search flag logic #946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
10 tasks done
albertz opened this issue Feb 13, 2022 · 1 comment · Fixed by #947
Closed
10 tasks done

Simpler search flag logic #946

albertz opened this issue Feb 13, 2022 · 1 comment · Fixed by #947

Comments

@albertz
Copy link
Member

albertz commented Feb 13, 2022

For rwth-i6/returnn_common#18 and also in general, it makes sense to simplify the logic around the search flag. Currently it is a per-network flag. But in principle we could always enable it and have it only configurable for ChoiceLayer.

It becomes a bit tricky to not break old setups though. This issue is about collecting all involved issues. The PR #947 is about implementing this.

When the search flag is not explicitly given for ChoiceLayer, it needs to derive it in the same way as it is right now. This is:

  • RETURNN is started with task == "search" which leads to Engine.search which constructs the network with search flag. It is probably not a good idea to rely on the global task option though. Engine.search could also be called by external scripts. Or external scripts could even directly create TFNetwork(..., search_flag=True, ...). Maybe we need to keep the API compatible at this point.
  • Extra nets, via extra....: prefix syntax, when it includes the string "search".

Edit I will now only consider the use case of returnn-common (including but not limited to rwth-i6/returnn_common#18), where some of the following items can be ignored for now. If we want to work on them, we can open a new separate issue.

What we should check:

  • only_on_search layer option, handled in TFNetwork.construct_from_dict
  • Some workaround in LayerBase.transform_config_dict for target layer option uses search flag, although this is somewhat incorrect anyway
  • loss_only_on_non_search layer option, also handled in LayerBase.transform_config_dict
  • Many instances (layers, Rec loop code, etc) just checking the search flag to skip certain further checks on search beam. This should in principle not have any impact to just remove these checks.
  • BaseChoiceLayer
  • ChoiceLayer
  • DecideKeepBeamLayer
  • DecideLayer
  • SplitBatchBeamLayer
  • RecLayer.transform_source_and_axis takes time dim from target when search flag is not set. This assumes that we search over the target. At this point, we have not yet checked the subnet at all, so we would not know about any containing ChoiceLayer.
@albertz
Copy link
Member Author

albertz commented Feb 13, 2022

Note, for returnn-common (including but not limited to rwth-i6/returnn_common#18) we don't need to have all of this fully implemented. We will not use only_on_search or loss_only_on_non_search or the RecLayer target option or any other layer target option, so all these points can be ignored. We just need to be able to enable the search for a specific ChoiceLayer, independently whether the global net search flag is enabled.

albertz added a commit that referenced this issue Feb 14, 2022
Use net search flag only as default fall back
when search option is not explicitly provided for ChoiceLayer.
DecideLayer and co will always apply.

ChoiceLayer, new add_to_beam_scores option
to control whether the score should be added to the beam
when not doing search.

Fix #946.

Related:
rwth-i6/returnn_common#18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant