Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1046)

Issue 2391383005: [LanguageModel] Return top languages only with a reasonable sample set (Closed)

Created:
4 years, 2 months ago by jkrcal
Modified:
4 years, 2 months ago
Reviewers:
droger
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[LanguageModel] Return top languages only with a reasonable sample set Before, the LanguageModel returned top languages even if it had only one data point. Thus the frequencies of top languages could change drastically in the warm-up phase of the model. This CL adds a minimum size of the sample set. The model returns empty list of top languages before this minimal size is reached. BUG=653058 Committed: https://crrev.com/0de27079beae15d87fd0adb969ad536422031bf9 Cr-Commit-Position: refs/heads/master@{#423849}

Patch Set 1 #

Patch Set 2 : Unit-test fix #

Patch Set 3 : Fix the constructor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -17 lines) Patch
M components/translate/core/browser/language_model.h View 2 chunks +3 lines, -2 lines 0 comments Download
M components/translate/core/browser/language_model.cc View 1 2 3 chunks +11 lines, -3 lines 0 comments Download
M components/translate/core/browser/language_model_unittest.cc View 1 3 chunks +18 lines, -12 lines 0 comments Download

Messages

Total messages: 18 (11 generated)
jkrcal
David, could you PTAL at this small CL?
4 years, 2 months ago (2016-10-07 10:32:37 UTC) #2
droger
Looks good, but it seems you need to update the unittest as well.
4 years, 2 months ago (2016-10-07 10:58:22 UTC) #7
jkrcal
On 2016/10/07 10:58:22, droger wrote: > Looks good, but it seems you need to update ...
4 years, 2 months ago (2016-10-07 11:22:51 UTC) #10
droger
lgtm
4 years, 2 months ago (2016-10-07 12:16:30 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2391383005/40001
4 years, 2 months ago (2016-10-07 12:21:38 UTC) #15
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 2 months ago (2016-10-07 14:07:24 UTC) #16
commit-bot: I haz the power
4 years, 2 months ago (2016-10-07 14:08:53 UTC) #18
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/0de27079beae15d87fd0adb969ad536422031bf9
Cr-Commit-Position: refs/heads/master@{#423849}

Powered by Google App Engine
This is Rietveld 408576698