browsermt / bergamot-translator

Cross platform C++ library focusing on optimized machine translation on the consumer-grade device.
http://browser.mt
Mozilla Public License 2.0
340 stars 38 forks source link

Allow per-input options #346

Closed jerinphilip closed 2 years ago

jerinphilip commented 2 years ago

Changes signature of BlockingService::{translate,pivot}Multiple functions to take per input options, so a mix of HTML and plaintext can be sent from the extension. Templating over testing is adjusted to allow for continuous evaluations by modifying the test code.

Updates WebAssembly bindings to reflect the change in signature and the javascript test-page to work with the new bindings.

This change lacks an accompanying test specific to the mixed HTML and plaintext inputs.

Fixes: #345 See also: https://github.com/mozilla/firefox-translations/issues/94

jerinphilip commented 2 years ago

Check c5cea4b (#346) for a quick confirmation this feature works. In the interest of speeding up Mozilla, I'm proposing to merge this at the earliest.

For inputs mixed sent with corresponding HTML response-options:

std::vector<std::string> plainTexts = {
      "Hello World!",                                 //
      "The quick brown fox jumps over the lazy dog."  //
  };

  std::vector<std::string> htmlTexts = {
      "<a href=\"#\">Hello</a> world.",                                                   //
      "The quick brown <b id=\"fox\">fox</b> jumps over the lazy <i id=\"dog\">dog</i>."  //
  };

The library code works:

Hallo Welt!
Der schnelle braune Fuchs springt über den faulen Hund.
<a href="#">Hallo</a> Welt.
Der schnelle braune <b id="fox">Fuchs</b> springt über den faul<i id="dog">en Hund.</i>

The final tests will follow at a later stage when I can access var and can be designed better.

abhi-agg commented 2 years ago

@jerinphilip Do you intend to make a similar change to Async service as well?

jerinphilip commented 2 years ago

Do you intend to make a similar change to Async service as well?

Not sure how this is relevant to Mozilla efforts, but I will try to answer. AsyncService was meant to operate at one input which allows one option setting, not wait and go forward. Batching will happen by queueing and not blocking. Also figured it's easier to provide cancellation handles per request than for a batch of requests. Maybe we will change it later if we need the concept of a "BatchJob", but not right now. Where such a change relevant is perhaps python, but I don't see an immediate need to do it unless there's a client.

jerinphilip commented 2 years ago

@jelmervdl points out (in internal messaging) without VectorResponseOptions at WASM bindings the code is broken for use in JS, consequently, JS is broken as well. But the required C++ API changes have been completed.

@abhi-agg Please note that I expect you to experiment with and bring changes to worker.js and bindings necessary as you would usually do in the verification step before communicating to extension dev.

jerinphilip commented 2 years ago

@jelmervdl has been kind enough to edit the WASM/HTML bindings as well. Requesting @abhi-agg to review now.