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

allow_partial for intersect_dense_pruned #1087

Closed

Conversation

glynpu
Copy link
Contributor

@glynpu glynpu commented Nov 3, 2022

Following results are tested with a deliberately selected input and decoding configuration.

with allow_partial=False (the original code):

There is no final arc(i.e. arc.label == -1).

0 1 0 0 -8.41582e-05                                                                                                                                                        
1 2 0 0 -4.69674e-05                                                                                                                                                        
2 3 0 0 -9.17907e-06                                                                                                                                                        
3 4 0 0 -1.10864e-05                                                                                                                                                        
4 5 0 0 -1.4305e-05                                                                                                                                                         
5 6 0 0 -2.63449e-05                                                                                                                                                        
6 7 0 0 -6.55649e-06                                                                                                                                                        
7 8 0 0 -1.19209e-05                                                                                                                                                        
8 9 0 0 -3.12323e-05                                                                                                                                                        
9 10 0 0 -2.36032e-05                                                                                                                                                       
11

with allow_partial=True (current pr):

image

k2/python/k2/autograd.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@pkufool pkufool left a comment

Choose a reason for hiding this comment

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

I recommend we implement allow_partial only within the FormatOutput function without modifying code in other functions, I think it is possible, I did not test it by myself, though.

if (allow_partial_) {
arc.label = -1;
} else {
// Unreachable code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this branch unreachable, if I understand correctly, this branch should do nothing instead of raising fatal error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For arcs pointing to super-final state, their labels must be -1 if allow_partial==false.
Just add this "else" branch to trigger some un-realized bug in the future.

int32_t a_fsas_final_state_idx1 = a_fsas_row_splits1[a_fsas_idx0 + 1] - 1 - a_fsas_row_splits1[a_fsas_idx0];
dest_state = a_fsas_final_state_idx1;
acoustic_score = 0.0;
}
Copy link
Collaborator

@pkufool pkufool Nov 4, 2022

Choose a reason for hiding this comment

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

I think this block and the above block are not necessary, we can know which sequence has no final state by the shape of ArcInfo at the last frame (see first K2_EVAL in FormatOutput), at the last frame, there will be only one state (the final state) or no state at all.
Another thing, if we modify ai.u.dest_a_fsas_state_idx01 here, it will mess the arc-info and might raise an error for chunk by chunk decoding. Actually, if we know which sequcence has no final-arc, we can set the dest-state of the arcs at the last frame to the extra state we added (see first K2_EVAL in FormatOutput) without knowing ai.u.dest_info_state_idx1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last frame log_probs is manually added [0.0, -inf, -inf, -inf, ..., -inf, -inf].
The main purpose of this block is setting those -inf to 0.0.
Or all active arcs will be pruned by function PruneTimeRange

This is also the reason when the input num_frames == 20, while the generated lattice length is only 10!
Not only the final arc is missing, but also the last "10" frames are pruned by PruneTimeRange .

Before fix, lattice length is 10.

0 1 0 0 -8.41582e-05                                                                                                                                                        
1 2 0 0 -4.69674e-05                                                                                                                                                        
2 3 0 0 -9.17907e-06                                                                                                                                                        
3 4 0 0 -1.10864e-05                                                                                                                                                        
4 5 0 0 -1.4305e-05                                                                                                                                                         
5 6 0 0 -2.63449e-05                                                                                                                                                        
6 7 0 0 -6.55649e-06                                                                                                                                                        
7 8 0 0 -1.19209e-05                                                                                                                                                        
8 9 0 0 -3.12323e-05                                                                                                                                                        
9 10 0 0 -2.36032e-05                                                                                                                                                       
11

After fix the length is 20.
image

@pkufool
Copy link
Collaborator

pkufool commented Sep 23, 2023

Merged in #1218

@pkufool pkufool closed this Sep 23, 2023
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.

4 participants