Improves TranslatorExpansions with pre-processing and post-processing #2213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This provides an improvement to the TranslatorExpansions model. Now, it includes three types of expansions: pre-processing expansions, post-processing expansions, and full translator expansions. Before, it only had the full translator expansions. The key improvement is that any combination of pre-processing expansion and post-processing expansion can be used. So, if there are 5 pre-processing and 3 post-processing then it will be 15 expansions. It also follows a standard tendency to handle the pre-processing and post-processing separately as seen with classes like the BaseImageTranslatorFactory. Let me talk about the model for the expansion a bit. The translator can be thought of as the composition of 5 different functions: pre-processing -> batchify -> model -> unbatchify -> post-process. With an expansion, it increases this to 7: pre-processing expansion -> pre-processing -> batchify -> model -> unbatchify -> post-process -> post-processing expansion. The weakness of this implementation is that it also limits it to 7. For example, we can use a Translator<InputStream -> Classifications> and expand the pre-processing to form the Translator<Url -> Classifications>. With this model, there is no built-in way to do this. That being said, this manifests mostly in increased work on the side of writing expansions rather than a loss in functionality. You would just have to use the InputStreamImagePreProcessor as part of the UrlImagePreProcessor or otherwise write it from scratch. In exchange for increased simplicity over a more complete model, I think it is a worthwhile tradeoff. Following this creation, there are a few other changes which I will list here for ease of reading: 1. It creates a BasicTranslator which simply composes a separate PreProcessor and PostProcessor. It is a useful utility in it's own right 2. Moves the FileTranslator, UrlTranslator, and InputStreamTranslator into just a PreProcessor 3. Uses the new ExpansionFactory to update the BaseImageTranslatorFactory and apply it where possible
Codecov ReportBase: 72.08% // Head: 71.62% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2213 +/- ##
============================================
- Coverage 72.08% 71.62% -0.47%
- Complexity 5126 6329 +1203
============================================
Files 473 631 +158
Lines 21970 27972 +6002
Branches 2351 2980 +629
============================================
+ Hits 15838 20034 +4196
- Misses 4925 6479 +1554
- Partials 1207 1459 +252
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
frankfliu
approved these changes
Dec 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This provides an improvement to the TranslatorExpansions model. Now, it includes three types of expansions: pre-processing expansions, post-processing expansions, and full translator expansions. Before, it only had the full translator expansions.
The key improvement is that any combination of pre-processing expansion and post-processing expansion can be used. So, if there are 5 pre-processing and 3 post-processing then it will be 15 expansions. It also follows a standard tendency to handle the pre-processing and post-processing separately as seen with classes like the BaseImageTranslatorFactory.
Let me talk about the model for the expansion a bit. The translator can be
thought of as the composition of 5 different functions: pre-processing ->
batchify -> model -> unbatchify -> post-process.
With an expansion, it increases this to 7: pre-processing expansion ->
pre-processing -> batchify -> model -> unbatchify -> post-process -> post-processing expansion.
The weakness of this implementation is that it also limits it to 7. For example, we can use a Translator<InputStream -> Classifications> and expand the pre-processing to form the Translator<Url -> Classifications>. With this model, there is no built-in way to do this. That being said, this manifests mostly in increased work on the side of writing expansions rather than a loss in functionality. You would just have to use the InputStreamImagePreProcessor as part of the UrlImagePreProcessor or otherwise write it from scratch. In exchange for increased simplicity over a more complete model, I think it is a worthwhile tradeoff.
Following this creation, there are a few other changes which I will list here for ease of reading: