Allow per-input options (#346)

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: mozilla/firefox-translations#94 Co-authored-by: Jelmer van der Linde <[email protected]>
browsermt · Feb 11, 2022 · ec46919 · ec46919
1 parent 3478652
commit ec46919
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 24 deletions.
diff --git a/src/tests/common-impl.cpp b/src/tests/common-impl.cpp
@@ -8,7 +8,8 @@ Response Bridge<BlockingService>::translate(BlockingService &service, std::share
   // project source to a vector of std::string, send in, unpack the first element from
   // vector<Response>, return.
   std::vector<std::string> sources = {source};
-  return service.translateMultiple(model, std::move(sources), responseOptions).front();
+  std::vector<ResponseOptions> options = {responseOptions};
+  return service.translateMultiple(model, std::move(sources), options).front();
 }
 
 Response Bridge<AsyncService>::translate(AsyncService &service, std::shared_ptr<TranslationModel> &model,
@@ -30,7 +31,8 @@ Response Bridge<BlockingService>::pivot(BlockingService &service, std::shared_pt
                                         std::shared_ptr<TranslationModel> &pivotToTarget, std::string &&source,
                                         const ResponseOptions &responseOptions) {
   std::vector<std::string> sources = {source};
-  return service.pivotMultiple(sourceToPivot, pivotToTarget, std::move(sources), responseOptions).front();
+  std::vector<ResponseOptions> options = {responseOptions};
+  return service.pivotMultiple(sourceToPivot, pivotToTarget, std::move(sources), options).front();
 }
 
 Response Bridge<AsyncService>::pivot(AsyncService &service, std::shared_ptr<TranslationModel> &sourceToPivot,

diff --git a/src/tests/wasm.cpp b/src/tests/wasm.cpp
@@ -2,7 +2,7 @@
 using namespace marian::bergamot;
 
 void wasm(BlockingService &service, std::shared_ptr<TranslationModel> &model) {
-  ResponseOptions responseOptions;
+  std::vector<ResponseOptions> responseOptions;
   std::vector<std::string> texts;
 
   // WASM always requires HTML and alignment.
@@ -13,6 +13,7 @@ void wasm(BlockingService &service, std::shared_ptr<TranslationModel> &model) {
   // Hide the translateMultiple operation
   for (std::string line; std::getline(std::cin, line);) {
     texts.emplace_back(line);
+    responseOptions.emplace_back();
   }
 
   auto results = service.translateMultiple(model, std::move(texts), responseOptions);

diff --git a/src/translator/service.cpp b/src/translator/service.cpp
@@ -41,10 +41,10 @@ BlockingService::BlockingService(const BlockingService::Config &config)
 
 std::vector<Response> BlockingService::translateMultiple(std::shared_ptr<TranslationModel> translationModel,
                                                          std::vector<std::string> &&sources,
-                                                         const ResponseOptions &responseOptions) {
+                                                         const std::vector<ResponseOptions> &responseOptions) {
   std::vector<HTML> htmls;
-  for (auto &&source : sources) {
-    htmls.emplace_back(std::move(source), responseOptions.HTML);
+  for (size_t i = 0; i < sources.size(); i++) {
+    htmls.emplace_back(std::move(sources[i]), responseOptions[i].HTML);
   }
   std::vector<Response> responses = translateMultipleRaw(translationModel, std::move(sources), responseOptions);
   for (size_t i = 0; i < responses.size(); i++) {
@@ -56,15 +56,15 @@ std::vector<Response> BlockingService::translateMultiple(std::shared_ptr<Transla
 
 std::vector<Response> BlockingService::translateMultipleRaw(std::shared_ptr<TranslationModel> translationModel,
                                                             std::vector<std::string> &&sources,
-                                                            const ResponseOptions &responseOptions) {
+                                                            const std::vector<ResponseOptions> &responseOptions) {
   std::vector<Response> responses;
   responses.resize(sources.size());
 
   for (size_t i = 0; i < sources.size(); i++) {
     auto callback = [i, &responses](Response &&response) { responses[i] = std::move(response); };  //
     TranslationCache *cache = config_.cacheEnabled ? &cache_ : nullptr;
     Ptr<Request> request =
-        translationModel->makeRequest(requestId_++, std::move(sources[i]), callback, responseOptions, cache);
+        translationModel->makeRequest(requestId_++, std::move(sources[i]), callback, responseOptions[i], cache);
     batchingPool_.enqueueRequest(translationModel, request);
   }
 
@@ -80,10 +80,10 @@ std::vector<Response> BlockingService::translateMultipleRaw(std::shared_ptr<Tran
 std::vector<Response> BlockingService::pivotMultiple(std::shared_ptr<TranslationModel> first,
                                                      std::shared_ptr<TranslationModel> second,
                                                      std::vector<std::string> &&sources,
-                                                     const ResponseOptions &responseOptions) {
+                                                     const std::vector<ResponseOptions> &responseOptions) {
   std::vector<HTML> htmls;
-  for (auto &&source : sources) {
-    htmls.emplace_back(std::move(source), responseOptions.HTML);
+  for (size_t i = 0; i < sources.size(); i++) {
+    htmls.emplace_back(std::move(sources[i]), responseOptions[i].HTML);
   }
 
   // Translate source to pivots. This is same as calling translateMultiple.
@@ -103,7 +103,7 @@ std::vector<Response> BlockingService::pivotMultiple(std::shared_ptr<Translation
 
     TranslationCache *cache = config_.cacheEnabled ? &cache_ : nullptr;
     Ptr<Request> request =
-        second->makePivotRequest(requestId_++, std::move(intermediate), callback, responseOptions, cache);
+        second->makePivotRequest(requestId_++, std::move(intermediate), callback, responseOptions[i], cache);
     batchingPool_.enqueueRequest(second, request);
   }
 

diff --git a/src/translator/service.h b/src/translator/service.h
@@ -57,30 +57,34 @@ class BlockingService {
 
   /// @param [in] translationModel: TranslationModel to use for the request.
   /// @param [in] source: rvalue reference of the string to be translated
-  /// @param [in] responseOptions: ResponseOptions indicating whether or not to include some member in the Response,
-  /// also specify any additional configurable parameters.
+  /// @param [in] responseOptions: ResponseOptions per source-item indicating whether or not to include some member in
+  /// the Response, also specify any additional configurable parameters.
   std::vector<Response> translateMultiple(std::shared_ptr<TranslationModel> translationModel,
-                                          std::vector<std::string> &&source, const ResponseOptions &responseOptions);
+                                          std::vector<std::string> &&source,
+                                          const std::vector<ResponseOptions> &responseOptions);
 
-  std::vector<Response> translateMultipleRaw(std::shared_ptr<TranslationModel> translationModel,
-                                             std::vector<std::string> &&source, const ResponseOptions &responseOptions);
   /// With the supplied two translation models, translate using first and then the second generating a response as if it
   /// were translated from first's source language to second's target langauge. Requires first's target to be second's
   /// source to work correctly - effectively implementing pivoting translation via an intermediate language.
   ///
   /// @param[in] first: TranslationModel capable of translating from source language to pivot language.
   /// @param[in] second: TranslationModel capable of translating between pivot and target language.
   /// @param[move] sources: The input source texts to be translated.
-  /// @param[in] options: Options indicating whether or not to include optional members in response and pass additional
-  /// configurations. See ResponseOptions.
+  /// @param[in] options: Options indicating whether or not to include optional members per source-text. See
+  /// ResponseOptions.
   ///
   /// @returns responses corresponding to the source-text which can be used as if they were translated with
   /// translateMultiple.
   std::vector<Response> pivotMultiple(std::shared_ptr<TranslationModel> first, std::shared_ptr<TranslationModel> second,
-                                      std::vector<std::string> &&sources, const ResponseOptions &responseOptions);
+                                      std::vector<std::string> &&sources,
+                                      const std::vector<ResponseOptions> &responseOptions);
   TranslationCache::Stats cacheStats() { return cache_.stats(); }
 
  private:
+  std::vector<Response> translateMultipleRaw(std::shared_ptr<TranslationModel> translationModel,
+                                             std::vector<std::string> &&source,
+                                             const std::vector<ResponseOptions> &responseOptions);
+
   ///  Numbering requests processed through this instance. Used to keep account of arrival times of the request. This
   ///  allows for using this quantity in priority based ordering.
   size_t requestId_;

diff --git a/wasm/bindings/response_options_bindings.cpp b/wasm/bindings/response_options_bindings.cpp
@@ -17,4 +17,5 @@ EMSCRIPTEN_BINDINGS(response_options) {
       .field("qualityScores", &ResponseOptions::qualityScores)
       .field("alignment", &ResponseOptions::alignment)
       .field("html", &ResponseOptions::HTML);
+  register_vector<ResponseOptions>("VectorResponseOptions");
 }
diff --git a/wasm/test_page/js/worker.js b/wasm/test_page/js/worker.js
@@ -119,16 +119,16 @@ const translate = (from, to, input) => {
 
   // Prepare the arguments (ResponseOptions and vectorSourceText (vector<string>)) of Translation API and call it.
   // Result is a vector<Response> where each of its item corresponds to one item of vectorSourceText in the same order.
-  const responseOptions = _prepareResponseOptions();
+  const vectorResponseOptions = _prepareResponseOptions();
   let vectorSourceText = _prepareSourceText(input);
   let vectorResponse;
   if (translationModels.length == 2) {
     // This implies translation should be done via pivoting
-    vectorResponse = translationService.translateViaPivoting(translationModels[0], translationModels[1], vectorSourceText, responseOptions);
+    vectorResponse = translationService.translateViaPivoting(translationModels[0], translationModels[1], vectorSourceText, vectorResponseOptions);
   }
   else {
     // This implies translation should be done without pivoting
-    vectorResponse = translationService.translate(translationModels[0], vectorSourceText, responseOptions);
+    vectorResponse = translationService.translate(translationModels[0], vectorSourceText, vectorResponseOptions);
   }
 
   // Parse all relevant information from vectorResponse
@@ -146,6 +146,7 @@ const translate = (from, to, input) => {
 
   // Delete prepared SourceText to avoid memory leak
   vectorSourceText.delete();
+  vectorResponseOptions.delete();
 
   return listTranslatedText;
 }
@@ -341,7 +342,9 @@ const _parseTranslatedTextSentenceQualityScores = (vectorResponse) => {
 }
 
 const _prepareResponseOptions = () => {
-  return {qualityScores: true, alignment: true, html: true};
+  const vector = new Module.VectorResponseOptions();
+  vector.push_back({qualityScores: true, alignment: true, html: true})
+  return vector;
 }
 
 const _prepareSourceText = (input) => {