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

implement RmEpsilonPrunedLogSum #47

Merged
merged 1 commit into from
May 29, 2020
Merged

Conversation

qindazhu
Copy link
Collaborator

No description provided.

@danpovey
Copy link
Collaborator

you can merge if you want

@qindazhu
Copy link
Collaborator Author

Ok, merging.

@qindazhu qindazhu merged commit 4a44429 into k2-fsa:master May 29, 2020
b->arc_indexes.reserve(num_states_b + 1);
int32_t arc_num_b = 0;

const double *forward_state_weights = a.ForwardStateWeights();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qindazhu I notice that your code tends to be a bit unstructured and hard to read. Compare with my code in determinize.cc... see that I mostly have shorter functions and I spend a lot of time documenting things. Just think about it for the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, actually I plan to refactor the code after implemented RmEpsMax/LogSum(non pruned version, so there are 4 similar function). Of course, will notice the documentation issue. Thanks!

@qindazhu qindazhu deleted the haowen-eps branch May 29, 2020 06:56
@@ -50,6 +52,41 @@ inline int32_t InsertIntersectionState(
return result.first->second;
}

static void TraceBackRmEpsilonLogSum(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare this with how carefully I document functions like this in determinize.cc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will add in another PR.

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.

2 participants