Extending 'lattice-compose.cc' to compose with ark of fsts, #4692

KarelVesely84 · 2022-01-25T19:45:08Z

This is a follow-up of: On the fly contextual adaptation, #4571
Refactoring 'lattice-compose.cc' to support composition with ark
of fsts, so that it is done as Dan suggested before:

  I am thinking this can be done with a string arg called e.g.
  "--compose-with-fst", defaulting to "auto" which is the old behavior,
  meaning: rspecifier=lats, rxfilename=FST; and true/True or false/False
  is FST or lattice respectively.

I added there possibility of rho-composition, which is useful for
biasing lattices with word-sequences. Thanks to rho-composition,
the biasing graph does not need to contain all words from lexicon.
Would you be interested in an example how to use this?
(i.e. create graphs from text file with python script
using openfst as library, but that would need to change
build of openfst to enable python extensions)
Also which 'egs' recipe would be convenient to use it with?

- This is a follow-up of kaldi-asr#4571 - Refactoring 'lattice-compose.cc' to support composition with ark of fsts, so that it is done as Dan suggested before: I am thinking this can be done with a string arg called e.g. "--compose-with-fst", defaulting to "auto" which is the old behavior, meaning: rspecifier=lats, rxfilename=FST; and true/True or false/False is FST or lattice respectively. - I added there possibility of rho-composition, which is useful for biasing lattices with word-sequences. Thanks to rho-composition, the biasing graph does not need to contain all words from lexicon. - Would you be interested in an example how to use this? (i.e. create graphs from text file with python script using openfst as library, but that would need to change build of openfst to enable python extensions) - Also which 'egs' recipe would be convenient to use it with?

danpovey · 2022-01-26T02:08:51Z

src/latbin/lattice-compose.cc

@@ -121,7 +164,23 @@ int main(int argc, char *argv[]) {
        }
      }
      delete fst2;
-    } else {
+    }


Thanks!

Would you mind formatting the else in a more standard way that won't upset auto-formatters?
E.g.

} else { /* comment here */ }

.. I see why you did what you did, but let's be consistent with other code.
Are you confident that this won't break existing code?

danpovey · 2022-01-26T03:46:40Z

src/latbin/lattice-compose.cc

@@ -39,22 +39,34 @@ int main(int argc, char *argv[]) {
        "or lattices with FSTs (rspecifiers are assumed to be lattices, and\n"
        "rxfilenames are assumed to be FSTs, which have their weights interpreted\n"
        "as \"graph weights\" when converted into the Lattice format.\n"
+        "Or, rspecifier can be ark of biasing FSTs, see --compose-with-fst=true.\n"


This whole message is not very clear, especially with the modification. Can write:

Depending on the command-line arguments, either composes lattices with lattices,
or lattices with a single FST or multiple FSTs (whose weights are interpreted as "graph weights").

Then change the lines below to:

Usage: lattice-compose [options] <lattice-rspecifier1> " "<lattice-rspecifier2|fst-rxfilename2|fst-rspecifier2> <lattice-wspecifier>\n" If the 2nd arg is an rspecifier, it is interpreted by default as a table of lattices, or as a table of FSTs if you specify --compose-with-fst=true.

csukuangfj · 2022-01-26T04:31:28Z

src/latbin/lattice-compose.cc

+    { // convert 'compose_with_fst' to lowercase to support: true, True, TRUE
+      std::string tmp_lc(compose_with_fst);
+      std::transform(compose_with_fst.begin(), compose_with_fst.end(),
+                     tmp_lc.begin(), ::tolower);  // lc


Suggested change

tmp_lc.begin(), ::tolower); // lc

compose_with_fst.begin(), std::tolower); // lc

It can be an in-place operation.

I see a strange behavior, the std::tolower version produces error:

/usr/local/include/c++/9.4.0/bits/stl_algo.h:4369:5: note: template argument deduction/substitution failed: lattice-compose.cc:87:71: note: candidate expects 5 arguments, 4 provided 87 | std::transform(str.begin(), str.end(), str.begin(), std::tolower); // lc

i found that i can go-around it by wrapping it into lambda:
[](unsigned char c){ return std::tolower(c);}
but that looks quite complicated compared to ::tolower which compiles well,
and i saw it used in: util/parse-options.cc and nnet/nnet-component.cc ...

csukuangfj · 2022-01-26T04:32:43Z

src/latbin/lattice-compose.cc

+     * arg2 is rspecifier that contains a table of lattices
+     * - composing arg1 lattices with arg2 lattices
+     */
+    else if (not arg2_is_rxfilename &&


Suggested change

else if (not arg2_is_rxfilename &&

else if (!arg2_is_rxfilename &&

jtrmal · 2022-01-27T17:40:35Z

was there a message why the 4-param std::transform cannot be used? I wonder if there should be push_back_iterator(tmp_lc.begin())? y.

…

On Thu, Jan 27, 2022 at 6:34 PM Karel Vesely ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/latbin/lattice-compose.cc <#4692 (comment)>: > @@ -63,14 +75,30 @@ int main(int argc, char *argv[]) { } KALDI_ASSERT(phi_label > 0 || phi_label == fst::kNoLabel); // e.g. 0 not allowed. + KALDI_ASSERT(rho_label > 0 || rho_label == fst::kNoLabel); // e.g. 0 not allowed. + if (phi_label > 0 && rho_label > 0) { + KALDI_ERR << "You cannot set both 'phi_label' and 'rho_label' at the same time."; + } + + { // convert 'compose_with_fst' to lowercase to support: true, True, TRUE + std::string tmp_lc(compose_with_fst); + std::transform(compose_with_fst.begin(), compose_with_fst.end(), + tmp_lc.begin(), ::tolower); // lc I see a strange behavior, the std::tolower version produces error: /usr/local/include/c++/9.4.0/bits/stl_algo.h:4369:5: note: template argument deduction/substitution failed: lattice-compose.cc:87:71: note: candidate expects 5 arguments, 4 provided 87 | std::transform(str.begin(), str.end(), str.begin(), std::tolower); // lc i found that i can go-around it by wrapping it into lambda: [](unsigned char c){ return std::tolower(c);} but that looks quite complicated compared to ::tolower which compiles well, and i saw it used in: util/parse-options.cc and nnet/nnet-component.cc ... — Reply to this email directly, view it on GitHub <#4692 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUKYXZH7LLKQPUXGTLGEFDUYF6Y5ANCNFSM5MZFUJEA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because your review was requested.Message ID: ***@***.***>

KarelVesely84 · 2022-01-27T17:48:33Z

it seems like the symbol std::tolower is overloaded, there is unary variant we want:
int tolower( int ch ) in <cctype>
the other is two-arg:
charT tolower( charT ch, const locale& loc ) from <locale>
so, the std::transform does not know how to resolve this... (if we wanted unary or two-arg functor)

KarelVesely84 · 2022-01-27T17:50:30Z

the messages are:

/usr/local/include/c++/9.4.0/bits/stl_algo.h:4332:5: note:   template argument deduction/substitution failed:
lattice-compose.cc:84:71: note:   couldn’t deduce template parameter ‘_UnaryOperation’
   84 |       std::transform(str.begin(), str.end(), str.begin(), std::tolower);  // lc

and

/usr/local/include/c++/9.4.0/bits/stl_algo.h:4369:5: note:   template argument deduction/substitution failed:
lattice-compose.cc:84:71: note:   candidate expects 5 arguments, 4 provided
   84 |       std::transform(str.begin(), str.end(), str.begin(), std::tolower);  // lc

KarelVesely84 · 2022-01-27T18:04:54Z

okay, i'll put there the lambda, i did not find any better solution...
(like wrapping std::tolower to resolve the overloading...)

KarelVesely84 · 2022-01-27T18:12:46Z

interesting, this one works too:
std::transform(str.begin(), str.end(), str.begin(), (int(*)(int))std::tolower); // lc

…of overloaded function

KarelVesely84 · 2022-01-27T18:22:31Z

For the testing, i did test the variant: ark:lattices - ark:fsts, RhoComposition, and it works well.

For the other combinations ark:lattices - ark:lattices, ark:lattices - fst and did not test it,
but i did not change their code that was already there (just RhoCompose was added in the if-then-else).
And there are 3 composition types: Compose, PhiCompose and RhoCompose.

So, out of these 9 combinations, should i test everything?
Or, just some of them?

KarelVesely84 · 2022-01-27T18:24:19Z

also, normally, in lattices there is normally no phi or rho symbol, but the PR code allows it...
(could there be possibly someone introducing phi/rho into a lattice manually ?)

KarelVesely84 · 2022-01-31T10:19:58Z

I did some of the tests. I did the variants:

lattice o fsts (rho composition, phi composition)
lattice o 1fst (rho composition)
lattices o lattices

In all these cases it produces lattice ark of about the same size as the input.
Plus for "lattice o fsts (rho composition)" it produced 1% WER improvement
on my air-traffic communication task.

Otherwise, all the previous remarks to the code sholud be resolved now.
K.

danpovey · 2022-01-31T10:26:20Z

src/latbin/lattice-compose.cc

    po.Register("write-compact", &write_compact, "If true, write in normal (compact) form.");
    po.Register("phi-label", &phi_label, "If >0, the label on backoff arcs of the LM");
+    po.Register("rho-label", &rho_label,
+                "If >0, the label to forward fst1 paths not present in biasing graph fst2 "
+                "(rho is input and output symbol on special arc in biasing graph)");


perhaps could specify that rho is like phi but the label is rewritten to the specific symbol.

danpovey · 2022-01-31T13:06:54Z

OK, I'm going to merge this so that we have some forward movement.
It might be more ideal to have an example script, but we can work on that later.

KarelVesely84 requested review from danpovey, jtrmal and kkm000 January 25, 2022 19:45

KarelVesely84 self-assigned this Jan 25, 2022

danpovey reviewed Jan 26, 2022

View reviewed changes

csukuangfj requested changes Jan 26, 2022

View reviewed changes

lattice-compose.cc, resolving remarks from PR kaldi-asr#4692

f185670

fixing issue in std::transform with std::tolower, suggesting variant …

7dc3723

…of overloaded function

danpovey reviewed Jan 31, 2022

View reviewed changes

lattice-compose, extending the rho explanation

cb6f272

danpovey merged commit fe230a0 into kaldi-asr:master Jan 31, 2022

KarelVesely84 mentioned this pull request May 10, 2022

On the fly contextual adaptation, #4571

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extending 'lattice-compose.cc' to compose with ark of fsts, #4692

Extending 'lattice-compose.cc' to compose with ark of fsts, #4692

KarelVesely84 commented Jan 25, 2022

danpovey Jan 26, 2022

danpovey Jan 26, 2022

csukuangfj Jan 26, 2022

KarelVesely84 Jan 27, 2022

csukuangfj Jan 26, 2022

jtrmal commented Jan 27, 2022 via email

KarelVesely84 commented Jan 27, 2022 •

edited

Loading

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022 •

edited

Loading

KarelVesely84 commented Jan 27, 2022 •

edited

Loading

KarelVesely84 commented Jan 31, 2022

danpovey Jan 31, 2022

KarelVesely84 Jan 31, 2022

danpovey commented Jan 31, 2022

	tmp_lc.begin(), ::tolower); // lc
	compose_with_fst.begin(), std::tolower); // lc

	else if (not arg2_is_rxfilename &&
	else if (!arg2_is_rxfilename &&

Extending 'lattice-compose.cc' to compose with ark of fsts, #4692

Extending 'lattice-compose.cc' to compose with ark of fsts, #4692

Conversation

KarelVesely84 commented Jan 25, 2022

danpovey Jan 26, 2022

Choose a reason for hiding this comment

danpovey Jan 26, 2022

Choose a reason for hiding this comment

csukuangfj Jan 26, 2022

Choose a reason for hiding this comment

KarelVesely84 Jan 27, 2022

Choose a reason for hiding this comment

csukuangfj Jan 26, 2022

Choose a reason for hiding this comment

jtrmal commented Jan 27, 2022 via email

KarelVesely84 commented Jan 27, 2022 • edited Loading

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022

KarelVesely84 commented Jan 27, 2022 • edited Loading

KarelVesely84 commented Jan 27, 2022 • edited Loading

KarelVesely84 commented Jan 31, 2022

danpovey Jan 31, 2022

Choose a reason for hiding this comment

KarelVesely84 Jan 31, 2022

Choose a reason for hiding this comment

danpovey commented Jan 31, 2022

KarelVesely84 commented Jan 27, 2022 •

edited

Loading

KarelVesely84 commented Jan 27, 2022 •

edited

Loading

KarelVesely84 commented Jan 27, 2022 •

edited

Loading