Build ctc graph from symbols in batch mode #776

pkufool · 2021-07-07T13:03:58Z

The python api and results are as follows:

pzelasko · 2021-07-07T13:10:09Z

Maybe a naive question -- what is the difference between this implementation, and doing it via Python-level k2 operations? I remember there was some code from @csukuangfj for MMI that batched all the FSA ops for a given list of utterances. Just curious what is the gain.

k2/csrc/fsa_algo.cu

k2/python/k2/__init__.py

k2/python/k2/fsa_algo.py

k2/python/tests/ctc_graph_test.py

csukuangfj · 2021-07-07T14:11:28Z

Maybe a naive question -- what is the difference between this implementation, and doing it via Python-level k2 operations? I remember there was some code from @csukuangfj for MMI that batched all the FSA ops for a given list of utterances. Just curious what is the gain.

@pzelasko
There are several differences that I can think of (there may be more):

(1) There are no optional silences before and after each word in this pull-request.
(2) This pull-request constructs the resulting graph directly, instead of creating it from k2.intersect. It is presumably faster
as it involves fewer kernel calls, though it needs benchmarks.
(3) For CTC training, we don't need to construct the graph ctc_topo anymore with this pull-request.
(4) The labels in this pull-request are input tokens, and its aux_labels are also tokens/epsilons. While in snowfall, the labels are phones and aux_labels are words/epsilons.
(5) In snowfall, it supports words with multiple pronunciations since there is a lexicon, while this pull-request
supports only one.
(6) In snowfall, it supports both MMI and CTC. This pull-request supports only CTC training since there is no bigram P.

csukuangfj · 2021-07-07T14:20:25Z

k2/csrc/fsa_algo.h

+  Create an FsaVec containing ctc graph FSAs, given a list of sequences of
+  symbols
+
+    @param [in] symbols  Input symbol sequences (must not contain


Do we need to add a check inside the kernel that none of the input symbols is -1?

I add a checking in the kernel set_num_arcs, which will enumerate all the symbols. I think it's enough.

k2/python/tests/ctc_graph_test.py

csukuangfj · 2021-07-08T00:39:40Z

k2/csrc/fsa_algo.cu

+                sym_state_idx01 = state_idx01 / 2 - fsa_idx0,
+                remainder = state_idx01 % 2,
+                current_num_arcs = 2;  // normally there are two arcs, self-loop
+                                       // and arc points to the next state


Suggested change

// and arc points to the next state

// and arc pointing to the next state

csukuangfj · 2021-07-08T00:44:41Z

k2/csrc/fsa_algo.cu

+          } else {
+            int32_t current_symbol = symbol_data[sym_state_idx01],
+                    // we set the next symbol of the last symbol to -1, so
+                    // the following if clause will always be true


Could you explain the comment:

so the following if clause will always be true

It means that the last symbol state would always have 3 arcs. we handle next_symbol here, first, to avoid segment fault error, second, to confirm that current_symbol != next_symbol so we will assign the last symbol state 3 arcs.

will explain more in the docs.

csukuangfj · 2021-07-08T00:45:34Z

k2/csrc/fsa_algo.cu

+      });
+
+  ExclusiveSum(num_states_for, &num_states_for);
+  Array1<int32_t> &fsa_to_states_row_splits = num_states_for;


Is there a reason to introduce another name for num_states_for?

After doing ExclusiveSum, num_states_for is actually the row_splits, using different names here just for easy understanding, we'll use fsa_to_states_row_splits to construct ragged_shape below.

csukuangfj · 2021-07-08T01:25:48Z

k2/python/csrc/torch/fsa_algo.cu

+        if (need_arc_map) tensor = ToTorch(arc_map);
+        return std::make_pair(graph, tensor);
+      },
+      py::arg("labels"), py::arg("need_arc_map") = true, py::arg("gpu_id"));


Suggested change

py::arg("labels"), py::arg("need_arc_map") = true, py::arg("gpu_id"));

py::arg("symbols"), py::arg("need_arc_map") = true, py::arg("gpu_id"));

csukuangfj · 2021-07-08T01:53:03Z

k2/python/csrc/torch/fsa_algo.cu

+  m.def(
+      "ctc_graph",
+      [](const Ragged<int32_t> &symbols, bool need_arc_map = true,
+         int32_t /*unused_gpu_id*/)


The last argument has no default value but it is after an argument with default value.
It's not valid C++, I think.

I saw the error in actions, it could compile successfully on linux, will fix it.

danpovey · 2021-07-08T02:37:20Z

I think there should be a boolean option to specify the type of CTC topology: "standard" or "simplified", where the "standard" one makes the blank mandatory between a pair of identical symbols.

pkufool · 2021-07-08T08:52:36Z

I think there should be a boolean option to specify the type of CTC topology: "standard" or "simplified", where the "standard" one makes the blank mandatory between a pair of identical symbols.

Ok, will add the option.

pkufool · 2021-07-08T09:34:18Z

Add the option standard, default True, the standard one makes the blank mandatory between a pair of identical symbols.

An example to demonstrate their difference is as follow:

danpovey · 2021-07-08T11:31:24Z

Great, thanks! Feel free to merge when you guys think it's OK.

…

On Thu, Jul 8, 2021 at 5:34 PM pkufool ***@***.***> wrote: Add the option standard, default True, the standard one makes the blank mandatory between a pair of identical symbols. An example to demonstrate their difference is as follow: [image: image] <https://user-images.githubusercontent.com/11765074/124899049-50872780-e012-11eb-9d85-96ea49344410.png> — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#776 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZFLO764YMISFYRLYEGCFTTWVWKJANCNFSM476T765Q> .

k2/csrc/fsa_algo.cu

csukuangfj · 2021-07-08T12:28:30Z

k2/csrc/fsa_algo.cu

+          // There is no arcs for final states
+          if (sym_state_idx01 == sym_final_state) {
+            current_num_arcs = 0;
+          } else {


Suggested change

} else {

} else if(!standard) {

current_num_arcs = 3;

} else {

// same as before the latest change

}

For non-standard topo, current_num_arcs is always 3. Put it into
a separate if statement can save some work.

k2/python/k2/fsa_algo.py

k2/csrc/fsa_algo.cu

Co-authored-by: Fangjun Kuang <[email protected]>

pkufool added 2 commits July 7, 2021 13:42

contructing ctc graphs from symbols

b9f09a3

add more documents and add python unit test

574d5f4

csukuangfj reviewed Jul 7, 2021

View reviewed changes

k2/python/tests/ctc_graph_test.py Show resolved Hide resolved

pkufool added 2 commits July 8, 2021 06:58

add rangged tensor unit test, fix code style

6387d2b

add symbol checking & handle the final symbol

8ee147a

csukuangfj reviewed Jul 8, 2021

View reviewed changes

add standard option to choose ctc topolopy

182c260

csukuangfj reviewed Jul 8, 2021

View reviewed changes

k2/csrc/fsa_algo.cu Outdated Show resolved Hide resolved

k2/csrc/fsa_algo.cu Outdated Show resolved Hide resolved

pkufool and others added 2 commits July 9, 2021 06:24

Apply suggestions from code review

a83ae90

Co-authored-by: Fangjun Kuang <[email protected]>

apply suggestions from code review

a1a7902

csukuangfj merged commit fd59f07 into k2-fsa:master Jul 8, 2021

pkufool deleted the ctc_graph branch July 8, 2021 22:51

pkufool mentioned this pull request Jul 9, 2021

Constructing ctc decoding graph in a batch k2-fsa/snowfall#225

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build ctc graph from symbols in batch mode #776

Build ctc graph from symbols in batch mode #776

pkufool commented Jul 7, 2021

pzelasko commented Jul 7, 2021

csukuangfj commented Jul 7, 2021 •

edited

Loading

csukuangfj Jul 7, 2021

pkufool Jul 7, 2021

csukuangfj Jul 8, 2021

csukuangfj Jul 8, 2021

pkufool Jul 8, 2021

csukuangfj Jul 8, 2021

pkufool Jul 8, 2021

csukuangfj Jul 8, 2021

csukuangfj Jul 8, 2021

pkufool Jul 8, 2021

danpovey commented Jul 8, 2021

pkufool commented Jul 8, 2021

pkufool commented Jul 8, 2021

danpovey commented Jul 8, 2021 via email

csukuangfj Jul 8, 2021

	// and arc points to the next state
	// and arc pointing to the next state

	py::arg("labels"), py::arg("need_arc_map") = true, py::arg("gpu_id"));
	py::arg("symbols"), py::arg("need_arc_map") = true, py::arg("gpu_id"));

-          } else {
+          } else if(!standard) {
+            current_num_arcs = 3;
+          } else {
+            // same as before the latest change
+          }

Build ctc graph from symbols in batch mode #776

Build ctc graph from symbols in batch mode #776

Conversation

pkufool commented Jul 7, 2021

pzelasko commented Jul 7, 2021

csukuangfj commented Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danpovey commented Jul 8, 2021

pkufool commented Jul 8, 2021

pkufool commented Jul 8, 2021

danpovey commented Jul 8, 2021 via email

Choose a reason for hiding this comment

csukuangfj commented Jul 7, 2021 •

edited

Loading