New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 893425 link

Starred by 2 users

Issue metadata

Status: Available
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ML Service: Port smart-dim

Project Member Reported by amoylan@chromium.org, Oct 9

Issue description

Today smart-dim calls a TF native model. Let's replace this with a call to ML Service.

At a high level:
* Replace synchronous call to TFNative with async call to ML Service
* Verify that ML Service starts from cold and loads model and provides response quickly enough
* Expand smart-dim logging to cover the possibility of ML Service not returning in time

We may need to add a basic integration test here. A tast test.

 
Cc: pmalani@chromium.org
Another output of this effort can be to test, refine, and slightly expand, our client instructions:
https://chromium.git.corp.google.com/chromiumos/platform2/+/master/ml/README.md#how-to-use-ml-service
The TFNative model is invoked here at tfnative_model::Inference:

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/power/ml/smart_dim/model_impl.cc?g=0&l=242
napper@ mentioned this is a P0 for the Chrome OS ML service team.

Perhaps another bug should be spawned for palm-reject?
Cc: amoylan@chromium.org
Owner: pmalani@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/529233823616ce2cbe5a5ca113c93922a86fb329

commit 529233823616ce2cbe5a5ca113c93922a86fb329
Author: Prashant Malani <pmalani@chromium.org>
Date: Sat Dec 08 16:44:07 2018

ml: Add initial Smart DIM model.

This model was created from TOCO without the "--post_training_quantize"
option.

BUG=chromium:893425
TEST=cros_run_unit_tests

Change-Id: I902da362f9c51b8e2568985bbb0463715dbfa130
Reviewed-on: https://chromium-review.googlesource.com/1335250
Commit-Ready: Prashant Malani <pmalani@chromium.org>
Tested-by: Prashant Malani <pmalani@google.com>
Tested-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>

[modify] https://crrev.com/529233823616ce2cbe5a5ca113c93922a86fb329/chromeos-base/ml/ml-9999.ebuild
[modify] https://crrev.com/529233823616ce2cbe5a5ca113c93922a86fb329/chromeos-base/ml/Manifest

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90879a28e0f13b345a413b27bfe3d9ba3b73612f

commit 90879a28e0f13b345a413b27bfe3d9ba3b73612f
Author: Prashant Malani <pmalani@chromium.org>
Date: Thu Dec 13 05:15:39 2018

[Power ML]: Move Smart Dim call to new TaskRunner

Move the call to perform Smart Dim ML inference to a separate
TaskRunner. Also, modify UserActivityManager such that the result of the
inference is acted upon via callback instead of sychronously.

This is in preparation for a possible move to using the ML service, which
runs asynchronous inference calls that we don't want to block on the UI
thread.

Bug: 893425
Test: Builds and boots, inference calls checked via logs on nocturne.

Change-Id: I809f96da227272ae8640534ca58cb2645d1e23db
Reviewed-on: https://chromium-review.googlesource.com/c/1354036
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Jia Meng <jiameng@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616218}
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/smart_dim/model.h
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/smart_dim/model_impl.cc
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/smart_dim/model_impl.h
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/smart_dim/model_unittest.cc
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/user_activity_manager.cc
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/user_activity_manager.h
[modify] https://crrev.com/90879a28e0f13b345a413b27bfe3d9ba3b73612f/chrome/browser/chromeos/power/ml/user_activity_manager_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4c4bc7dc38f2f4913808f97af39da89380f6f1f2

commit 4c4bc7dc38f2f4913808f97af39da89380f6f1f2
Author: Andrew Moylan <amoylan@chromium.org>
Date: Tue Dec 18 04:47:05 2018

ml: Document .cc changes needed for new models

This CL expands the model publishing documentation to also describe the
changes needed to the ML Service daemon .cc code, using pmalani's
SMART_DIM port as the guiding example.

BUG= chromium:862807 ,chromium:893425
TEST=no

Change-Id: I24b469314dbd314c57476bee215badf42abf327e
Reviewed-on: https://chromium-review.googlesource.com/1379631
Commit-Ready: Andrew Moylan <amoylan@chromium.org>
Tested-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Claudio M <claudiomagni@chromium.org>

[modify] https://crrev.com/4c4bc7dc38f2f4913808f97af39da89380f6f1f2/ml/docs/publish_model.md

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6f2de07e28ec038b2e8757c58a751873eb018f90

commit 6f2de07e28ec038b2e8757c58a751873eb018f90
Author: Prashant Malani <pmalani@chromium.org>
Date: Tue Dec 18 08:42:52 2018

ml: Add support for Smart Dim model

Needs ebuild change to install the tflite model.

BUG=chromium:893425
TEST=Unit tests
CQ-DEPEND=CL:1335250

Change-Id: I287046d444863081c87f6ed8ddf1eaa2e9045d7b
Reviewed-on: https://chromium-review.googlesource.com/1342736
Commit-Ready: Prashant Malani <pmalani@google.com>
Tested-by: Prashant Malani <pmalani@google.com>
Reviewed-by: Prashant Malani <pmalani@google.com>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>

[modify] https://crrev.com/6f2de07e28ec038b2e8757c58a751873eb018f90/ml/model_metadata.cc
[modify] https://crrev.com/6f2de07e28ec038b2e8757c58a751873eb018f90/ml/machine_learning_service_impl_test.cc
[modify] https://crrev.com/6f2de07e28ec038b2e8757c58a751873eb018f90/ml/mojom/model.mojom
[modify] https://crrev.com/6f2de07e28ec038b2e8757c58a751873eb018f90/ml/machine_learning_service_impl.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90cb54cdbaf21c149654b36bee2d7c43a572612e

commit 90cb54cdbaf21c149654b36bee2d7c43a572612e
Author: Prashant Malani <pmalani@chromium.org>
Date: Fri Dec 28 00:13:05 2018

[Power: ML]: Add call to ML service for Smart Dim

Add support for using the Chrome OS ML service to perform an inference
for Smart Dim.

This CL updates the corresponding Mojo interface definition file to
reflect the addition of a new model. Additionally, it adds a new
preprocessor config file so that the inputs can be organized into the
correct vector for consumption by the new ML model.

Finally, it adds a client class which manages the calls to the ML
service and returns the results (or errors) back to the
calling code.

This feature is currently hidden behind flag:
  "UserActivityPredictionMlService".

BUG=chromium:893425
TEST=Build and deploy on a nocturne and verify that Inference calls
     complete as expected.

Change-Id: I71c55b5805da8c79ca125c1c10faf9e7ae97f959
Reviewed-on: https://chromium-review.googlesource.com/c/1352824
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Jia Meng <jiameng@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619097}
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/browser_resources.grd
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/BUILD.gn
[add] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/lite_example_preprocessor_config.pb
[add] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/ml_service_client.cc
[add] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/ml_service_client.h
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/model_impl.cc
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/model_impl.h
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/smart_dim/model_unittest.cc
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chrome/browser/chromeos/power/ml/user_activity_manager.cc
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chromeos/chromeos_features.cc
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chromeos/chromeos_features.h
[modify] https://crrev.com/90cb54cdbaf21c149654b36bee2d7c43a572612e/chromeos/services/machine_learning/public/mojom/model.mojom

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f

commit a0c1844467ad9e40e0a7ab242c8a09beeb4e998f
Author: Prashant Malani <pmalani@chromium.org>
Date: Wed Jan 02 22:12:35 2019

Add RenderViewHostTestHarness ctor for thread bundle

Add a new constructor for RenderViewHostTestHarness (and its derived
class ChromeRenderViewHostTestHarness) to include
ScopedTaskEnvironment parameters. This will allow the test harness
classes to use the ScopedTaskEnvironment's task runner for mock time
task runners, instead of instantiating a separate
TestMockTimeTaskRunner.

This CL also modifies the UserActivityManagerTest unit tests to leverage
this in-built ScopedTaskEnvironment and remove the mock time task
runner, and also makes the FakeSmartDimModel implementation utilize the
task runner for posting tasks, to better mimick real-world behaviour.

Bug: 893425,  914640 ,  917580 
Test: - Builds and boots, inference calls checked via logs on nocturne
      - All related unit tests still pass.

Change-Id: I552ae98f6b2bce88845d17648303001a3c644788
Reviewed-on: https://chromium-review.googlesource.com/c/1389963
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Jia Meng <jiameng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619512}
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/browser/chromeos/power/ml/adaptive_screen_brightness_manager_unittest.cc
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/browser/chromeos/power/ml/fake_boot_clock.cc
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/browser/chromeos/power/ml/fake_boot_clock.h
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/browser/chromeos/power/ml/user_activity_manager_unittest.cc
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/test/base/chrome_render_view_host_test_harness.cc
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/chrome/test/base/chrome_render_view_host_test_harness.h
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/a0c1844467ad9e40e0a7ab242c8a09beeb4e998f/content/public/test/test_renderer_host.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c29a991d2542140a8e8a30e894832172c8042596

commit c29a991d2542140a8e8a30e894832172c8042596
Author: Prashant Malani <pmalani@chromium.org>
Date: Fri Jan 04 06:28:26 2019

[Power: ML]: Add UMA logging for ML service Smart Dim calls

Add UMA logging of time taken to complete ML service requests,
as well as the time spent before a pending request is cancelled.
Also update the existing Smart Dim unit tests to verify that the logging
works.

Bug: 893425,  914640 
Test: - Builds and boots, inference calls checked via logs on nocturne
      - All related unit tests still pass.

Change-Id: I542af752a0133939c65eaa435ed3f5be907140db
Reviewed-on: https://chromium-review.googlesource.com/c/1393670
Reviewed-by: Jia Meng <jiameng@chromium.org>
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619892}
[modify] https://crrev.com/c29a991d2542140a8e8a30e894832172c8042596/chrome/browser/chromeos/power/ml/user_activity_manager.cc
[modify] https://crrev.com/c29a991d2542140a8e8a30e894832172c8042596/chrome/browser/chromeos/power/ml/user_activity_manager.h
[modify] https://crrev.com/c29a991d2542140a8e8a30e894832172c8042596/chrome/browser/chromeos/power/ml/user_activity_manager_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e8907ef95230b2f2c0aa771e4a9f3754339c60f

commit 1e8907ef95230b2f2c0aa771e4a9f3754339c60f
Author: Prashant Malani <pmalani@chromium.org>
Date: Mon Jan 07 06:21:04 2019

[Power: ML]: Add unit tests to check canceled Smart Dim request

This CL adds tests for a couple of scenarios where previously requested
Smart Dim decisions are canceled, and verifies that the events are
logged to UMA correctly.

Bug: 893425,  914640 
Test: - Builds and boots, inference calls checked via logs on nocturne.
      - All related unit tests still pass.

Change-Id: Ia412209360ea8f65b0087a7c87042a6cb243d7d5
Reviewed-on: https://chromium-review.googlesource.com/c/1395962
Reviewed-by: Jia Meng <jiameng@chromium.org>
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620260}
[modify] https://crrev.com/1e8907ef95230b2f2c0aa771e4a9f3754339c60f/chrome/browser/chromeos/power/ml/user_activity_manager.cc
[modify] https://crrev.com/1e8907ef95230b2f2c0aa771e4a9f3754339c60f/chrome/browser/chromeos/power/ml/user_activity_manager_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 20 (3 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42d3bd6995b12e00320f7bdc404816d324c01a66

commit 42d3bd6995b12e00320f7bdc404816d324c01a66
Author: Prashant Malani <pmalani@chromium.org>
Date: Sun Jan 20 06:57:26 2019

Add histograms for Smart Dim ML service calls

Bug: 893425
Test: Builds successfully.

Change-Id: I774f1e52f2111fffa13fb2a5d64427ecedc9591f
Reviewed-on: https://chromium-review.googlesource.com/c/1419266
Auto-Submit: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Jia Meng <jiameng@chromium.org>
Commit-Queue: Prashant Malani <pmalani@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624485}
[modify] https://crrev.com/42d3bd6995b12e00320f7bdc404816d324c01a66/tools/metrics/histograms/histograms.xml

Sign in to add a comment