Callback on Request completion to construct Response #65
Labels
cleanup
Something that can be refactored or better organized
mod: marian
Changes affecting marian-dev component
The changes proposed in this issue is to be implemented if approved following Alignments PR (#46), and can help #56, #53.
tl;dr:
f(x; \theta) = ResponseBuilder(histories; vocabs, std::promise<Response>)
It's a bit unnatural to have
std::promise<Response>
andvocabs
inRequest
, which should only hold request information.bergamot-translator/src/translator/request.h
Lines 38 to 40 in 317433a
Similarly
Response
is constructed fromHistories
andVocabs
(both of which needn't be there, #53). This can be simplified / neatened by having aResponseBuilder
outsideResponse
, breaking the following messy constructor into meaningful units.bergamot-translator/src/translator/response.cpp
Lines 11 to 103 in 317433a
ResponseBuilder
will handle taking inhistories
and be initialized withvocabs
and thepromise
. Usinghistories
andvocab
, moving the present constructor ofResponse
intoResponseBuilder
can enable the construction of aResponse
there, following which thestd::promise<Response>
can be set with the newly constructed instance ofResponse
.Response
will thus end up carrying only data members (AnnotatedBlob
s of source, target.QualityScore
s,Alignments
).Response
should become very thin, and in theory ready for WASM export (conditioned onAnnotation
being exportable). Nostring_view
offending,alignments
and stubQualityScores
ready.ResponseBuilder
initialized with vocabs and promise and accepting histories additionally can be registered as a callback toRequest
instead of the existing spread mechanism (to be fired once translation of request is completed), consolidating the transition from processedRequest
->Response
logic into this callback.QualityEstimation
people can be pointed towards justResponseBuilder
, where they will have additional access tohistories
to just operate and vocabs etc. Feels like a saner API.std::promise
can be replaced withstd::promise
equivalent of the enhanced future.The text was updated successfully, but these errors were encountered: