Skip to content
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

Make label loops optional in CtcTreeBuilder for RNA topology support #100

Open
wants to merge 6 commits into
base: uncouple-treebuilder
Choose a base branch
from

Conversation

larissakl
Copy link
Contributor

As discussed in #99, the difference between the CtcTreeBuilder and a tree builder for Transducer/RNA topology is minimal and a dedicated TransducerTreeBuilder subclass can be avoided.
Therefore I introduced a member labelLoops_ to CtcTreeBuilder, which determines whether self-loops on the non-blank labels are added (CTC topology) or not (RNA topology).
Depends on #98.

@larissakl
Copy link
Contributor Author

Update:

I introduced the RnaTreeBuilder class as a subclass of CtcTreeBuilder. In SearchSpace.cc, there is now a new config-parameter, allow-label-loop.

  • If allow-label-loop is not set, it defaults to true for CtcTreeBuilder ("classic" CTC topology) and false for RnaTreeBuilder ("classic" RNA topology).
  • If tree-builder-type = rna and allow-label-loop = yes, the RnaTreeBuilder class is still used, but it will effectively build a tree with CTC topology.
  • Conversely, if tree-builder-type = ctc and allow-label-loop = no, the CtcTreeBuilder will build a tree with RNA topology.

I’m still unsure about the code style and the intended behavior, so feel free to share feedback or make any necessary changes directly.

@larissakl larissakl requested a review from SimBe195 February 24, 2025 14:12
@curufinwe
Copy link
Contributor

Move the allow-label-loop parameter into CtcTreeBuilder and also add it to RnaTreeBuilder, but with a different default value. Then in the RnaTreeBuilder call the parent constructor and afterwards initialize the allowLabelLoop_ member again with the parameter that is inside RnaTreeBuilder.

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 this pull request may close these issues.

3 participants