From b4006afaeedab196db10be5d993b5a610107e375 Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 14:49:59 +0000 Subject: [PATCH 1/8] Allow per-input options --- src/tests/common-impl.cpp | 6 ++++-- src/tests/wasm.cpp | 2 +- src/translator/service.cpp | 18 +++++++++--------- src/translator/service.h | 9 ++++++--- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/tests/common-impl.cpp b/src/tests/common-impl.cpp index 238e3acd6..43b1c07d2 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 9a29a20e1..71373b4bb 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 = {ResponseOptions()}; std::vector texts; // WASM always requires HTML and alignment. diff --git a/src/translator/service.cpp b/src/translator/service.cpp index 6ac1fd100..8d887b53a 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 87054e2cc..c8434f1cb 100644 --- a/src/translator/service.h +++ b/src/translator/service.h @@ -60,10 +60,12 @@ class BlockingService { /// @param [in] responseOptions: ResponseOptions 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); + std::vector &&source, + const std::vector &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. @@ -77,7 +79,8 @@ class BlockingService { /// @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: From c056eb4a7be6fda91a4d8f256cea4fb153c782bc Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 15:28:51 +0000 Subject: [PATCH 2/8] Oops, this needed to be same count as the text feed --- src/tests/wasm.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tests/wasm.cpp b/src/tests/wasm.cpp index 71373b4bb..97f0fc801 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) { - std::vector 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); From c5cea4b21170cac793d9f9d93b35fa5687e28dfc Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 15:59:22 +0000 Subject: [PATCH 3/8] Use mix of HTML and plaintext in test --- src/tests/wasm.cpp | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/tests/wasm.cpp b/src/tests/wasm.cpp index 97f0fc801..a1072646a 100644 --- a/src/tests/wasm.cpp +++ b/src/tests/wasm.cpp @@ -2,21 +2,32 @@ using namespace marian::bergamot; void wasm(BlockingService &service, std::shared_ptr &model) { - std::vector responseOptions; - std::vector texts; + std::vector plainTexts = { + "Hello World!", // + "The quick brown fox jumps over the lazy dog." // + }; + + std::vector htmlTexts = { + "Hello world.", // + "The quick brown fox jumps over the lazy dog." // + }; - // WASM always requires HTML and alignment. - // TODO(jerinphilip): Fix this, bring in actual tests. - // responseOptions.HTML = true; - // responseOptions.alignment = true; // Necessary for HTML + std::vector texts; + std::vector options; + for (auto &plainText : plainTexts) { + texts.push_back(plainText); + ResponseOptions opt; + options.push_back(opt); + } - // Hide the translateMultiple operation - for (std::string line; std::getline(std::cin, line);) { - texts.emplace_back(line); - responseOptions.emplace_back(); + for (auto &htmlText : htmlTexts) { + texts.push_back(htmlText); + ResponseOptions opt; + opt.HTML = true; + options.push_back(opt); } - auto results = service.translateMultiple(model, std::move(texts), responseOptions); + auto results = service.translateMultiple(model, std::move(texts), options); for (auto &result : results) { std::cout << result.getTranslatedText() << std::endl; From 92ea668665c67d104a90b06cc4f4d000b37d9748 Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 16:52:44 +0000 Subject: [PATCH 4/8] Revert "Use mix of HTML and plaintext in test" This reverts commit c5cea4b21170cac793d9f9d93b35fa5687e28dfc. --- src/tests/wasm.cpp | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/tests/wasm.cpp b/src/tests/wasm.cpp index a1072646a..97f0fc801 100644 --- a/src/tests/wasm.cpp +++ b/src/tests/wasm.cpp @@ -2,32 +2,21 @@ using namespace marian::bergamot; void wasm(BlockingService &service, std::shared_ptr &model) { - std::vector plainTexts = { - "Hello World!", // - "The quick brown fox jumps over the lazy dog." // - }; - - std::vector htmlTexts = { - "Hello world.", // - "The quick brown fox jumps over the lazy dog." // - }; - + std::vector responseOptions; std::vector texts; - std::vector options; - for (auto &plainText : plainTexts) { - texts.push_back(plainText); - ResponseOptions opt; - options.push_back(opt); - } - for (auto &htmlText : htmlTexts) { - texts.push_back(htmlText); - ResponseOptions opt; - opt.HTML = true; - options.push_back(opt); + // WASM always requires HTML and alignment. + // TODO(jerinphilip): Fix this, bring in actual tests. + // responseOptions.HTML = true; + // responseOptions.alignment = true; // Necessary for HTML + + // 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), options); + auto results = service.translateMultiple(model, std::move(texts), responseOptions); for (auto &result : results) { std::cout << result.getTranslatedText() << std::endl; From a445f529e61a2cc6e8564cafdb19a4f7a863c9e9 Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 17:02:03 +0000 Subject: [PATCH 5/8] per-source-item in doc --- src/translator/service.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/translator/service.h b/src/translator/service.h index c8434f1cb..c7691d6fe 100644 --- a/src/translator/service.h +++ b/src/translator/service.h @@ -57,8 +57,8 @@ 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 std::vector &responseOptions); From c95e87094e2f3f7d3a5fa8ea5d1d8150c972b394 Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Thu, 10 Feb 2022 17:03:52 +0000 Subject: [PATCH 6/8] Update pivot docs to per source item remark --- src/translator/service.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/translator/service.h b/src/translator/service.h index c7691d6fe..d35eb20a8 100644 --- a/src/translator/service.h +++ b/src/translator/service.h @@ -73,8 +73,8 @@ 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. From 189af089f25d0e882eeeb670e0233991af3642ca Mon Sep 17 00:00:00 2001 From: Jerin Philip Date: Fri, 11 Feb 2022 10:20:44 +0000 Subject: [PATCH 7/8] Make translateMultipleRaw private --- src/translator/service.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/translator/service.h b/src/translator/service.h index d35eb20a8..d45fcc467 100644 --- a/src/translator/service.h +++ b/src/translator/service.h @@ -63,9 +63,6 @@ class BlockingService { std::vector &&source, const std::vector &responseOptions); - std::vector translateMultipleRaw(std::shared_ptr translationModel, - std::vector &&source, - const std::vector &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. @@ -84,6 +81,10 @@ class BlockingService { 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_; From 221427b59eb9c11b6e6c3a97f7f493d287df6fa1 Mon Sep 17 00:00:00 2001 From: Jelmer van der Linde Date: Fri, 11 Feb 2022 11:18:18 +0000 Subject: [PATCH 8/8] Add bindings for reponseOptions --- wasm/bindings/response_options_bindings.cpp | 1 + wasm/test_page/js/worker.js | 11 +++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/wasm/bindings/response_options_bindings.cpp b/wasm/bindings/response_options_bindings.cpp index c58d24c64..06c152a7c 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 d079c783f..3e315fb9a 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) => {