Cleanup API: Refactor request on-complete transition

app/service-cli-bytearray.cpp

@@ -27,8 +27,14 @@ int main(int argc, char *argv[]) {
   std::string input = std_input.str();
   using marian::bergamot::Response;
 
+  marian::Ptr<marian::Options> responseOptions = marian::New<marian::Options>();
+  responseOptions->set<bool>("quality", true);
+  responseOptions->set<bool>("alignment", true);
+  responseOptions->set<float>("alignment-threshold", 0.2f);
+
   // Wait on future until Response is complete
-  std::future<Response> responseFuture = service.translate(std::move(input));
+  std::future<Response> responseFuture =
+      service.translate(std::move(input), responseOptions);
   responseFuture.wait();
   Response response = responseFuture.get();
 

app/service-cli.cpp

@@ -21,8 +21,14 @@ int main(int argc, char *argv[]) {
   std::string input = std_input.str();
   using marian::bergamot::Response;
 
+  marian::Ptr<marian::Options> responseOptions = marian::New<marian::Options>();
+  responseOptions->set<bool>("quality", true);
+  responseOptions->set<bool>("alignment", true);
+  responseOptions->set<float>("alignment-threshold", 0.2f);
+
   // Wait on future until Response is complete
-  std::future<Response> responseFuture = service.translate(std::move(input));
+  std::future<Response> responseFuture =
+      service.translate(std::move(input), responseOptions);
   responseFuture.wait();
   Response response = responseFuture.get();
 

src/translator/CMakeLists.txt

@@ -6,7 +6,7 @@ add_library(bergamot-translator STATIC
     batch_translator.cpp 
     request.cpp 
     batcher.cpp
-    response.cpp
+    response_builder.cpp
     batch.cpp
     sentence_ranges.cpp
     service.cpp

src/translator/TranslationModel.cpp

@@ -9,6 +9,7 @@
 // All local project includes
 #include "TranslationModel.h"
 #include "translator/parser.h"
+#include "translator/response.h"
 #include "translator/service.h"
 
 TranslationModel::TranslationModel(const std::string &config,
@@ -21,31 +22,25 @@ TranslationModel::~TranslationModel() {}
 std::vector<TranslationResult>
 TranslationModel::translate(std::vector<std::string> &&texts,
                             TranslationRequest request) {
-  // Implementing a non-async version first. Unpleasant, but should work.
-  std::promise<std::vector<TranslationResult>> promise;
-  auto future = promise.get_future();
 
   // This code, move into async?
   std::vector<TranslationResult> translationResults;
-  for (auto &text : texts) {
-    // Collect future as marian::bergamot::TranslationResult
-    auto intermediate = service_.translate(std::move(text));
-    intermediate.wait();
-    auto marianResponse(std::move(intermediate.get()));
-
+  std::vector<marian::bergamot::Response> responses =
+      service_.translateMultiple(std::move(texts), request);
+  for (auto &response : responses) {
     TranslationResult::SentenceMappings sentenceMappings;
-    for (size_t idx = 0; idx < marianResponse.size(); idx++) {
-      marian::string_view src = marianResponse.source.sentence(idx);
-      marian::string_view tgt = marianResponse.target.sentence(idx);
+    for (size_t idx = 0; idx < response.size(); idx++) {
+      marian::string_view src = response.source.sentence(idx);
+      marian::string_view tgt = response.target.sentence(idx);
       sentenceMappings.emplace_back(std::string_view(src.data(), src.size()),
                                     std::string_view(tgt.data(), tgt.size()));
     }
 
     // In place construction.
     translationResults.emplace_back(
-        std::move(marianResponse.source.text), // &&marianResponse.source_
-        std::move(marianResponse.target.text), // &&marianResponse.translation_
-        std::move(sentenceMappings)            // &&sentenceMappings
+        std::move(response.source.text), // &&response.source_
+        std::move(response.target.text), // &&response.translation_
+        std::move(sentenceMappings)      // &&sentenceMappings
     );
   }
 

src/translator/batch_translator.cpp

@@ -63,11 +63,14 @@ void BatchTranslator::translate(Batch &batch) {
   std::vector<data::SentenceTuple> batchVector;
 
   auto &sentences = batch.sentences();
+  size_t batchSequenceNumber{0};
   for (auto &sentence : sentences) {
-    data::SentenceTuple sentence_tuple(sentence.lineNumber());
+    data::SentenceTuple sentence_tuple(batchSequenceNumber);
     Segment segment = sentence.getUnderlyingSegment();
     sentence_tuple.push_back(segment);
     batchVector.push_back(sentence_tuple);
+
+    ++batchSequenceNumber;
   }
 
   size_t batchSize = batchVector.size();

src/translator/request.cpp

@@ -11,18 +11,17 @@ namespace marian {
 namespace bergamot {
 
 // -----------------------------------------------------------------
-Request::Request(size_t Id, size_t lineNumberBegin,
-                 std::vector<Ptr<Vocab const>> &vocabs, AnnotatedText &&source,
-                 Segments &&segments, std::promise<Response> responsePromise)
-    : Id_(Id), lineNumberBegin_(lineNumberBegin), vocabs_(&vocabs),
-      source_(std::move(source)), segments_(std::move(segments)),
-      response_(std::move(responsePromise)) {
+Request::Request(size_t Id, Segments &&segments,
+                 ResponseBuilder &&responseBuilder)
+    : Id_(Id), segments_(std::move(segments)),
+      responseBuilder_(std::move(responseBuilder))
+
+{
 
   counter_ = segments_.size();
   histories_.resize(segments_.size(), nullptr);
 }
 
-size_t Request::lineNumberBegin() const { return lineNumberBegin_; }
 size_t Request::numSegments() const { return segments_.size(); }
 
 size_t Request::segmentTokens(size_t index) const {
@@ -39,17 +38,10 @@ void Request::processHistory(size_t index, Ptr<History> history) {
   // In case this is last request in, completeRequest is called, which sets the
   // value of the promise.
   if (--counter_ == 0) {
-    completeRequest();
+    responseBuilder_(std::move(histories_));
   }
 }
 
-void Request::completeRequest() {
-  // Request no longer needs to hold the content, can transfer it to
-  // Response.
-  Response response(std::move(source_), std::move(histories_), *vocabs_);
-  response_.set_value(std::move(response));
-}
-
 bool Request::operator<(const Request &b) const {
   // Among Requests, only sequence id is used for obtaining priority.
   return Id_ < b.Id_;
@@ -64,10 +56,6 @@ size_t RequestSentence::numTokens() const {
   return (request_->segmentTokens(index_));
 }
 
-size_t RequestSentence::lineNumber() const {
-  return (request_->lineNumberBegin() + index_);
-}
-
 void RequestSentence::completeSentence(Ptr<History> history) {
   // Relays completeSentence into request's processHistory, using index
   // information.

src/translator/request.h

@@ -1,24 +1,9 @@
-//
-// Defines:
-//
-// Request: holds the input text of a text, Segments (vector<Words>) which are
-// to go to the batching mechanism and alignments between the processed
-// segments and the input text (sourceTokenRanges). In addition, Request takes
-// care of the barrier which fires when all the Segments in a request are done
-// translating by the workers (BatchTranslator).
-// TODO(jerinphilip):  Extend Request with notions of Priority (sequence,
-// user-given).
-//
-// RequestSentence: is a tuple of (index, Ptr<Request>). This provides the
-// batching mechanism access to the segment within the request. The backref to
-// Request allows event triggering the barrier upon completion of the last
-// sentence by a worker.
-
 #ifndef SRC_BERGAMOT_REQUEST_H_
 #define SRC_BERGAMOT_REQUEST_H_
 
 #include "definitions.h"
 #include "response.h"
+#include "response_builder.h"
 #include "sentence_ranges.h"
 
 #include "common/logging.h"
@@ -33,80 +18,96 @@
 namespace marian {
 namespace bergamot {
 
+/// A Request is an internal representation used to represent a request after
+/// processed by TextProcessor into sentences constituted by marian::Words.
+///
+/// The batching mechanism (Batcher) draws from multiple Requests and compiles
+/// sentences into a batch. When a batch completes translation (at
+/// BatchTranslator, intended in a different thread), backward propogation
+/// happens through:
+///
+/// ```cpp
+///   Batch::completeBatch(...)
+///       -> RequestSentence::completeSentence(..)
+///          -> Request::processHistory(...)
+/// ```
+///
+/// When all sentences in a Request are completed, responseBuilder is
+/// triggered with the compiled Histories, to construct the Response
+/// corresponding to the Request and set value of the promise which triggers the
+/// future at client.
 class Request {
 public:
-  Request(size_t Id, size_t lineNumberBegin,
-          std::vector<Ptr<Vocab const>> &vocabs_, AnnotatedText &&source,
-          Segments &&segments, std::promise<Response> responsePromise);
-
-  // Obtain the count of tokens in the segment correponding to index. Used to
-  // insert sentence from multiple requests into the corresponding size bucket.
+  /// Constructs an internal representation of the Request identified by Id,
+  /// processed Segments and accepts a callback (ResponseBuilder) which builds
+  /// the Response upon completion of the Request.
+  ///
+  ///
+  /// @param [in] Id: Identifier assigned to Request by Service.
+  /// @param [in] segments: Each segment is a unit to be translated.
+  /// @param [in] responseBuilder: Callback function (of ResponseBuilder type)
+  /// to be triggered upon the completion of translation of all units in a
+  /// Request.
+  Request(size_t Id, Segments &&segments, ResponseBuilder &&responseBuilder);
+
+  /// Obtain the count of tokens in the segment correponding to index. Used to
+  /// insert sentence from multiple requests into the corresponding size bucket.
   size_t segmentTokens(size_t index) const;
 
-  // Obtain number of segments in a request.
+  /// Obtain number of segments in a request.
   size_t numSegments() const;
-  size_t lineNumberBegin() const;
 
-  // Obtains segment corresponding to index  to create a batch of segments among
-  // several requests.
+  /// Obtains segment corresponding to index  to create a batch of segments
+  /// among several requests.
   Segment getSegment(size_t index) const;
 
-  // For notions of priority among requests, used to enable std::set in
-  // Batcher.
+  /// For notions of priority among requests, used to enable std::set in
+  /// Batcher.
   bool operator<(const Request &request) const;
 
-  // Processes a history obtained after translating in a heterogenous batch
-  // compiled from requests.
+  /// Processes a history obtained after translating in a heterogenous batch
+  /// compiled from requests.
   void processHistory(size_t index, Ptr<History> history);
 
-  // On completion of last segment, sets value of the promise.
-  void completeRequest();
-
 private:
   size_t Id_;
-  size_t lineNumberBegin_;
 
-  // Multiple translation-workers can concurrently access the same Request. The
-  // following atomic atomically operates on the variable holding sentences
-  // remaining to be translated.
+  /// Multiple translation-workers can concurrently access the same Request. The
+  /// following atomic atomically operates on the variable holding sentences
+  /// remaining to be translated.
   std::atomic<int> counter_;
 
-  // source_ holds the source string to be translated. segments_ hold the
-  // sentences generated from source_ in vector<Words>. sourceRanges_ are
-  // string_views of the text corresponding to these words, pointing to
-  // sequences in source_. histories_ is a buffer which eventually stores the
-  // translations of each segment in the corresponding index.
-  AnnotatedText source_;
+  /// segments_ hold the sentences processed into Words which generated from
+  /// input string.
   Segments segments_;
-  std::vector<Ptr<History>> histories_;
 
-  // Members above are moved into newly constructed Response on completion
-  // of translation of all segments. The promise below is set to this Response
-  // value. future to this promise is made available to the user through
-  // Service.
-  std::promise<Response> response_;
+  /// histories_ is a buffer which eventually stores the translations of each
+  /// segment in the corresponding index.
+  std::vector<Ptr<History>> histories_;
 
-  // Constructing Response requires the vocabs_ used to generate Request.
-  std::vector<Ptr<Vocab const>> *vocabs_;
+  /// Constructing Response requires the vocabs_ used to generate Request.
+  /// std::vector<Ptr<Vocab const>> *vocabs_;
+  ResponseBuilder responseBuilder_;
 };
 
+/// A RequestSentence provides a view to a sentence within a Request. Existence
+/// of this class allows the sentences and associated information to be kept
+/// within Request, while batching mechanism (Batcher) compiles Batch from
+/// RequestSentence-s coming from different Requests.
 class RequestSentence {
-  // A RequestSentence provides a view to a sentence within a Request. Existence
-  // of this class allows the sentences and associated information to be kept
-  // within Request.
 
 public:
   RequestSentence(size_t, Ptr<Request>);
-  size_t numTokens() const;
 
-  // lineNumber in Request, used for matching marian-decoder. SentenceTuple
-  // requires lineNumber to be set for Corpus based batches.
-  size_t lineNumber() const;
+  /// Number of tokens in the segment this RequestSentence represents. Used to
+  /// order by length in batching.
+  size_t numTokens() const;
 
-  // Accessor to the segment represented by the RequestSentence.
+  /// Accessor to the segment represented by the RequestSentence.
   Segment getUnderlyingSegment() const;
 
-  // Forwards call to Request, checking for completion.
+  /// Forwards history to Request to set history corresponding to this
+  /// RequestSentence.
   void completeSentence(Ptr<History> history);
 
   friend bool operator<(const RequestSentence &a, const RequestSentence &b);