Treebuilder: refactor trees_ vector #87
Conversation
|
I merged PR #85 . Could you @larissakl or @SimBe195 update the target branch with it so that we don't see the changes from #85 in this PR? Thanks! |
|
I merged the master branch into seq2seq-revamp, so the code from #85 should be included now |
# Conflicts: # src/Search/AdvancedTreeSearch/PersistentStateTree.cc # src/Search/AdvancedTreeSearch/TreeBuilder.cc
|
Hmm, the diff in GitHub still has a lot of the old changes. So something is not right yet. |
|
I see what you mean. Do you have any suggestions how to solve that? I'm not very good at Git, to be honest. |
It happens because this was created as a PR into seq2seq-revamp, not into master. Can you change the target of the PR to the master branch @larissakl ? |
|
Sure, but as I said, I'm not completely confident that everything is correct. I tested it with old |
We noted that the
trees_vector inHMMStateNetworkis actually superfluous because it always contains at most one element, making it unnecessary to save multipleTreeobjects. Therefore, I refactored this to use a single membertree_instead of a vector. As a result, constructs such as theTreeIndexand the concept of themasterTreeinPersistentStateTreecould also be removed.Since I'm not 100% confident that everything is correct and I want to avoid breaking anything, I have opened this PR to merge into the seq2seq-revamp branch. If you however believe the changes are fine, we could also consider merging it directly into the master branch
This PR builds on the code from PR #85 .