From ec469193c6a38b3dc2312eaf6304c83ea0ec0253 Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Fri, 11 Feb 2022 13:06:26 +0000 Subject: [PATCH] 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 --- src/tests/common-impl.cpp | 6 ++++-- src/tests/wasm.cpp | 3 ++- src/translator/service.cpp | 18 +++++++++--------- src/translator/service.h | 20 ++++++++++++-------- wasm/bindings/response_options_bindings.cpp | 1 + wasm/test_page/js/worker.js | 11 +++++++---- 6 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/tests/common-impl.cpp b/src/tests/common-impl.cpp index 238e3ac..43b1c07 100644 --- a/src/tests/common-impl.cpp +++ b/src/tests/common-impl.cpp @@ -8,7 +8,8 @@ Response Bridge::translate(BlockingService &service, std::share // project source to a vector of std::string, send in, unpack the first element from // vector, return. std::vector sources = {source}; - return service.translateMultiple(model, std::move(sources), responseOptions).front(); + std::vector options = {responseOptions}; + return service.translateMultiple(model, std::move(sources), options).front(); } Response Bridge::translate(AsyncService &service, std::shared_ptr &model, @@ -30,7 +31,8 @@ Response Bridge::pivot(BlockingService &service, std::shared_pt std::shared_ptr &pivotToTarget, std::string &&source, const ResponseOptions &responseOptions) { std::vector sources = {source}; - return service.pivotMultiple(sourceToPivot, pivotToTarget, std::move(sources), responseOptions).front(); + std::vector options = {responseOptions}; + return service.pivotMultiple(sourceToPivot, pivotToTarget, std::move(sources), options).front(); } Response Bridge::pivot(AsyncService &service, std::shared_ptr &sourceToPivot, diff --git a/src/tests/wasm.cpp b/src/tests/wasm.cpp index 9a29a20..97f0fc8 100644 --- 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 &model) { - ResponseOptions responseOptions; + std::vector responseOptions; std::vector texts; // WASM always requires HTML and alignment. @@ -13,6 +13,7 @@ void wasm(BlockingService &service, std::shared_ptr &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 index 6ac1fd1..8d887b5 100644 --- a/src/translator/service.cpp +++ b/src/translator/service.cpp @@ -41,10 +41,10 @@ BlockingService::BlockingService(const BlockingService::Config &config) std::vector BlockingService::translateMultiple(std::shared_ptr translationModel, std::vector &&sources, - const ResponseOptions &responseOptions) { + const std::vector &responseOptions) { std::vector 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 responses = translateMultipleRaw(translationModel, std::move(sources), responseOptions); for (size_t i = 0; i < responses.size(); i++) { @@ -56,7 +56,7 @@ std::vector BlockingService::translateMultiple(std::shared_ptr BlockingService::translateMultipleRaw(std::shared_ptr translationModel, std::vector &&sources, - const ResponseOptions &responseOptions) { + const std::vector &responseOptions) { std::vector responses; responses.resize(sources.size()); @@ -64,7 +64,7 @@ std::vector BlockingService::translateMultipleRaw(std::shared_ptr 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 BlockingService::translateMultipleRaw(std::shared_ptr BlockingService::pivotMultiple(std::shared_ptr first, std::shared_ptr second, std::vector &&sources, - const ResponseOptions &responseOptions) { + const std::vector &responseOptions) { std::vector 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 BlockingService::pivotMultiple(std::shared_ptr 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 index 87054e2..d45fcc4 100644 --- a/src/translator/service.h +++ b/src/translator/service.h @@ -57,13 +57,12 @@ 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 translateMultiple(std::shared_ptr translationModel, - std::vector &&source, const ResponseOptions &responseOptions); + std::vector &&source, + const std::vector &responseOptions); - std::vector translateMultipleRaw(std::shared_ptr translationModel, - std::vector &&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. @@ -71,16 +70,21 @@ class BlockingService { /// @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 pivotMultiple(std::shared_ptr first, std::shared_ptr second, - std::vector &&sources, const ResponseOptions &responseOptions); + std::vector &&sources, + const std::vector &responseOptions); TranslationCache::Stats cacheStats() { return cache_.stats(); } private: + std::vector translateMultipleRaw(std::shared_ptr translationModel, + std::vector &&source, + const std::vector &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 index c58d24c..06c152a 100644 --- 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("VectorResponseOptions"); } diff --git a/wasm/test_page/js/worker.js b/wasm/test_page/js/worker.js index d079c78..3e315fb 100644 --- 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)) of Translation API and call it. // Result is a vector 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) => {