[On-device adaptive brightness] Add constraints check by Trainer.

This cl contains the following changes
- Check whether initial personal curve satisfies min/max slope constraints.
- Add two params high_log_lux_threshold and min_grad_high_lux.
- Simplified trainer interface by removing status.
- Simplified modeller impl so that personal curve is kept by trainer only.

Bug: 881215
Change-Id: Iad8a16df3c14440ee9086124cfc8e3e2d890e5ca
Reviewed-on: https://chromium-review.googlesource.com/c/1303317
Commit-Queue: Jia Meng <jiameng@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604518}

chrome/browser/chromeos/power/auto_screen_brightness/gaussian_trainer.cc

@@ -122,63 +122,116 @@ double Gaussian(double x, double sigma) {
 
 }  // namespace
 
-// TODO(jiameng): move these params checking into another method and log errors
-// in UMA if any value is invalid. Also disable it if param isn't valid.
+GaussianTrainer::Params::Params() = default;
+
+// TODO(jiameng): log errors in UMA if any value is invalid.
 GaussianTrainer::GaussianTrainer() {
   params_.brightness_bound_scale = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "brightness_bound_scale",
       params_.brightness_bound_scale);
-  DCHECK_GT(params_.brightness_bound_scale, 0.0);
+  if (params_.brightness_bound_scale <= 0.0) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.brightness_bound_offset = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "brightness_bound_offset",
       params_.brightness_bound_offset);
-  DCHECK_GE(params_.brightness_bound_offset, 0.0);
+  if (params_.brightness_bound_offset < 0.0) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.brightness_step_size = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "brightness_step_size",
       params_.brightness_step_size);
-  DCHECK_GT(params_.brightness_step_size, 0.0);
+  if (params_.brightness_step_size <= 0.0) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.sigma = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "sigma", params_.sigma);
-  DCHECK_GT(params_.sigma, 0.0);
+  if (params_.sigma <= 0.0) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.low_log_lux_threshold = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "low_log_lux_threshold",
       params_.low_log_lux_threshold);
+  params_.high_log_lux_threshold = GetFieldTrialParamByFeatureAsDouble(
+      features::kAutoScreenBrightness, "high_log_lux_threshold",
+      params_.high_log_lux_threshold);
+  if (params_.low_log_lux_threshold >= params_.high_log_lux_threshold) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.min_grad_low_lux = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "min_grad_low_lux",
       params_.min_grad_low_lux);
+  params_.min_grad_high_lux = GetFieldTrialParamByFeatureAsDouble(
+      features::kAutoScreenBrightness, "min_grad_high_lux",
+      params_.min_grad_high_lux);
+
   params_.min_grad = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "min_grad", params_.min_grad);
   params_.max_grad = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "max_grad", params_.max_grad);
 
-  DCHECK_GE(params_.min_grad_low_lux, 0.0);
-  DCHECK_LT(params_.min_grad_low_lux, 1.0);
-  DCHECK_GE(params_.min_grad, 0.0);
-  DCHECK_LT(params_.min_grad, 1.0);
-  DCHECK_GE(params_.min_grad, params_.min_grad_low_lux);
-  DCHECK_GT(params_.max_grad, params_.min_grad);
+  if (params_.min_grad_low_lux < 0.0 || params_.min_grad_low_lux >= 1.0) {
+    valid_params_ = false;
+    return;
+  }
+
+  if (params_.min_grad_high_lux < 0.0 || params_.min_grad_high_lux >= 1.0) {
+    valid_params_ = false;
+    return;
+  }
+
+  if (params_.min_grad < 0.0 || params_.min_grad >= 1.0) {
+    valid_params_ = false;
+    return;
+  }
+
+  if (params_.min_grad < params_.min_grad_low_lux) {
+    valid_params_ = false;
+    return;
+  }
+
+  if (params_.min_grad < params_.min_grad_high_lux) {
+    valid_params_ = false;
+    return;
+  }
+
+  if (params_.max_grad < 1.0) {
+    valid_params_ = false;
+    return;
+  }
 
   params_.min_brightness = GetFieldTrialParamByFeatureAsDouble(
       features::kAutoScreenBrightness, "min_brightness",
       params_.min_brightness);
-  DCHECK_GE(params_.min_brightness, 0.0);
+  if (params_.min_brightness < 0.0) {
+    valid_params_ = false;
+    return;
+  }
 }
 
 GaussianTrainer::~GaussianTrainer() = default;
 
-// TODO(jiameng): add slope constraint check in |current_curve| and return check
-// result to the caller.
-void GaussianTrainer::SetInitialCurves(
+bool GaussianTrainer::HasValidConfiguration() const {
+  return valid_params_;
+}
+
+bool GaussianTrainer::SetInitialCurves(
     const MonotoneCubicSpline& global_curve,
     const MonotoneCubicSpline& current_curve) {
-  // This function should be called once only.
-  DCHECK(!global_curve_);
-  DCHECK(!current_curve_);
+  DCHECK(valid_params_);
+
+  // This function could be called again if the caller wants to reset the
+  // curves.
   global_curve_.emplace(global_curve);
   current_curve_.emplace(current_curve);
 
@@ -204,15 +257,39 @@ void GaussianTrainer::SetInitialCurves(
   DCHECK_GT(global_brightness[0], 0);
 
   for (size_t i = 0; i < num_points - 1; ++i) {
-    const double min_grad = global_log_lux[i] < params_.low_log_lux_threshold
-                                ? params_.min_grad_low_lux
-                                : params_.min_grad;
+    double min_grad = params_.min_grad;
+    if (global_log_lux[i] < params_.low_log_lux_threshold) {
+      min_grad = params_.min_grad_low_lux;
+    } else if (global_log_lux[i] > params_.high_log_lux_threshold) {
+      min_grad = params_.min_grad_high_lux;
+    }
 
     const double ratio = global_brightness[i + 1] / global_brightness[i];
     DCHECK_GE(ratio, 1);
     min_ratios_[i] = std::pow(ratio, min_grad);
     max_ratios_[i] = std::pow(ratio, params_.max_grad);
   }
+
+  if (!IsInitialPersonalCurveValid()) {
+    // Use global curve instead if personal curve isn't valid.
+    current_curve_.emplace(global_curve);
+    brightness_ = current_curve_->GetControlPointsY();
+    return false;
+  }
+
+  return true;
+}
+
+MonotoneCubicSpline GaussianTrainer::GetGlobalCurve() const {
+  DCHECK(valid_params_);
+  DCHECK(global_curve_);
+  return *global_curve_;
+}
+
+MonotoneCubicSpline GaussianTrainer::GetCurrentCurve() const {
+  DCHECK(valid_params_);
+  DCHECK(current_curve_);
+  return *current_curve_;
 }
 
 MonotoneCubicSpline GaussianTrainer::Train(
@@ -233,6 +310,20 @@ MonotoneCubicSpline GaussianTrainer::Train(
   return *current_curve_;
 }
 
+bool GaussianTrainer::IsInitialPersonalCurveValid() const {
+  // |global_curve_| is valid by construction.
+  if (*global_curve_ == *current_curve_)
+    return true;
+
+  for (size_t i = 0; i < brightness_.size() - 1; ++i) {
+    const double ratio = brightness_[i + 1] / brightness_[i];
+    if (ratio < min_ratios_[i] || ratio > max_ratios_[i])
+      return false;
+  }
+
+  return true;
+}
+
 void GaussianTrainer::AdjustCurveWithSingleDataPoint(
     const TrainingDataPoint& data) {
   const double brightness_global =

chrome/browser/chromeos/power/auto_screen_brightness/gaussian_trainer.h

@@ -23,6 +23,7 @@ class GaussianTrainer : public Trainer {
  public:
   // TODO(jiameng): revise default values.
   struct Params {
+    Params();
     // |brightness_bound_scale| and |brightness_bound_offset| are used to define
     // training example outliers.
     double brightness_bound_scale = 1.5;
@@ -45,6 +46,11 @@ class GaussianTrainer : public Trainer {
     double low_log_lux_threshold = 0.1;
     double min_grad_low_lux = 0;
 
+    // If log lux is above |high_log_lux_threshold| then we'll use
+    // |min_grad_high_lux| as gradient constraint.
+    double high_log_lux_threshold = 7.5;
+    double min_grad_high_lux = 0;
+
     // Min and max grad as a power of brightness ratios.
     double min_grad = 0.25;
     double max_grad = 1;
@@ -56,12 +62,19 @@ class GaussianTrainer : public Trainer {
   ~GaussianTrainer() override;
 
   // Trainer overrides:
-  void SetInitialCurves(const MonotoneCubicSpline& global_curve,
+  bool HasValidConfiguration() const override;
+  bool SetInitialCurves(const MonotoneCubicSpline& global_curve,
                         const MonotoneCubicSpline& current_curve) override;
+  MonotoneCubicSpline GetGlobalCurve() const override;
+  MonotoneCubicSpline GetCurrentCurve() const override;
   MonotoneCubicSpline Train(
       const std::vector<TrainingDataPoint>& data) override;
 
  private:
+  // Returns whether initial personal curve (passed in by |SetInitialCurves|) is
+  // valid, i.e. satisfying min/max ratio constraints.
+  bool IsInitialPersonalCurveValid() const;
+
   // Updates |brightness_| using |data|. It also sets |need_to_update_curve_|
   // to true if |brightness_| is actually changed.
   void AdjustCurveWithSingleDataPoint(const TrainingDataPoint& data);
@@ -72,6 +85,9 @@ class GaussianTrainer : public Trainer {
   // of |center_index|.
   void EnforceMonotonicity(size_t center_index);
 
+  // Default params_ are valid.
+  bool valid_params_ = true;
+
   Params params_;
   // |global_curve| does not change after |SetInitialCurves| is called.
   base::Optional<MonotoneCubicSpline> global_curve_;

chrome/browser/chromeos/power/auto_screen_brightness/modeller.h

@@ -16,16 +16,6 @@ namespace auto_screen_brightness {
 // Interface to on-device adaptive model.
 class Modeller {
  public:
-  // These values are persisted to logs. Entries should not be renumbered and
-  // numeric values should never be reused.
-  enum class Status {
-    kInitializing = 0,
-    kDisabled = 1,
-    kGlobal = 2,
-    kPersonal = 3,
-    kMaxValue = kPersonal
-  };
-
   // Modeller must outlive its observers.
   class Observer : public base::CheckedObserver {
    public:

chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.cc

@@ -118,7 +118,7 @@ ModellerImpl::~ModellerImpl() = default;
 void ModellerImpl::AddObserver(Modeller::Observer* observer) {
   DCHECK(observer);
   observers_.AddObserver(observer);
-  if (model_status_ != Modeller::Status::kInitializing) {
+  if (is_modeller_enabled_.has_value()) {
     NotifyObserverInitStatus(*observer);
   }
 }
@@ -129,7 +129,7 @@ void ModellerImpl::RemoveObserver(Modeller::Observer* observer) {
 }
 
 void ModellerImpl::OnAmbientLightUpdated(int lux) {
-  if (model_status_ == Status::kDisabled)
+  if (is_modeller_enabled_.has_value() && !*is_modeller_enabled_)
     return;
 
   const AmbientLightSample sample = {lux, tick_clock_->NowTicks()};
@@ -137,8 +137,7 @@ void ModellerImpl::OnAmbientLightUpdated(int lux) {
 }
 
 void ModellerImpl::OnAlsReaderInitialized(AlsReader::AlsInitStatus status) {
-  if (als_init_status_.has_value())
-    return;
+  DCHECK(!als_init_status_);
 
   als_init_status_ = status;
 
@@ -146,8 +145,7 @@ void ModellerImpl::OnAlsReaderInitialized(AlsReader::AlsInitStatus status) {
 }
 
 void ModellerImpl::OnBrightnessMonitorInitialized(bool success) {
-  if (brightness_monitor_success_.has_value())
-    return;
+  DCHECK(!brightness_monitor_success_.has_value());
 
   brightness_monitor_success_ = success;
   HandleStatusUpdate();
@@ -155,7 +153,7 @@ void ModellerImpl::OnBrightnessMonitorInitialized(bool success) {
 
 void ModellerImpl::OnUserBrightnessChanged(double old_brightness_percent,
                                            double new_brightness_percent) {
-  if (model_status_ == Status::kDisabled)
+  if (is_modeller_enabled_.has_value() && !*is_modeller_enabled_)
     return;
 
   // We don't add any training data if there is no ambient light sample.
@@ -236,13 +234,18 @@ ModellerImpl::ModellerImpl(
   DCHECK(user_activity_detector);
 
   if (!profile) {
-    model_status_ = Status::kDisabled;
+    is_modeller_enabled_ = false;
+    return;
+  }
+
+  if (!trainer_->HasValidConfiguration()) {
+    is_modeller_enabled_ = false;
     return;
   }
 
   curve_path_ = GetCurvePathFromProfile(profile);
   if (curve_path_.empty()) {
-    model_status_ = Status::kDisabled;
+    is_modeller_enabled_ = false;
     return;
   }
 
@@ -271,16 +274,16 @@ base::FilePath ModellerImpl::GetCurvePathFromProfile(Profile* profile) const {
 }
 
 void ModellerImpl::HandleStatusUpdate() {
-  if (model_status_ != Modeller::Status::kInitializing)
+  if (is_modeller_enabled_.has_value())
     return;
 
-  if (!als_init_status_.has_value()) {
+  if (!als_init_status_)
     return;
-  }
+
   const bool als_success =
-      als_init_status_.value() == AlsReader::AlsInitStatus::kSuccess;
+      *als_init_status_ == AlsReader::AlsInitStatus::kSuccess;
   if (!als_success) {
-    model_status_ = Modeller::Status::kDisabled;
+    is_modeller_enabled_ = false;
     OnInitializationComplete();
     return;
   }
@@ -288,8 +291,8 @@ void ModellerImpl::HandleStatusUpdate() {
   if (!brightness_monitor_success_.has_value()) {
     return;
   }
-  if (!brightness_monitor_success_.value()) {
-    model_status_ = Modeller::Status::kDisabled;
+  if (!*brightness_monitor_success_) {
+    is_modeller_enabled_ = false;
     OnInitializationComplete();
     return;
   }
@@ -309,28 +312,31 @@ void ModellerImpl::OnInitializationComplete() {
 }
 
 void ModellerImpl::NotifyObserverInitStatus(Modeller::Observer& observer) {
-  DCHECK_NE(model_status_, Status::kInitializing);
-  if (model_status_ == Status::kDisabled) {
+  DCHECK(is_modeller_enabled_.has_value());
+  if (!*is_modeller_enabled_) {
     observer.OnModelInitialized(base::nullopt, base::nullopt);
   } else {
-    observer.OnModelInitialized(global_curve_, personal_curve_);
+    base::Optional<MonotoneCubicSpline> personal_curve;
+    if (has_initial_personal_curve_)
+      personal_curve.emplace(trainer_->GetCurrentCurve());
+
+    observer.OnModelInitialized(global_curve_, personal_curve);
   }
 }
 
 void ModellerImpl::OnCurveLoadedFromDisk(
     const base::Optional<MonotoneCubicSpline>& curve) {
-  if (!curve.has_value()) {
-    model_status_ = Status::kGlobal;
-  } else {
-    personal_curve_.emplace(curve.value());
-    model_status_ = Status::kPersonal;
-  }
+  const bool is_personal_curve_valid =
+      trainer_->SetInitialCurves(global_curve_, curve ? *curve : global_curve_);
+
+  has_initial_personal_curve_ = is_personal_curve_valid && curve;
+  DCHECK(trainer_->GetGlobalCurve() == global_curve_);
+  DCHECK(trainer_->GetCurrentCurve() ==
+         (has_initial_personal_curve_ ? *curve : global_curve_));
 
+  is_modeller_enabled_ = true;
   OnInitializationComplete();
 
-  trainer_->SetInitialCurves(global_curve_, model_status_ == Status::kGlobal
-                                                ? global_curve_
-                                                : personal_curve_.value());
   ScheduleTrainerStart();
 }
 
@@ -356,7 +362,6 @@ void ModellerImpl::StartTraining() {
 }
 
 void ModellerImpl::OnTrainingFinished(const MonotoneCubicSpline& curve) {
-  personal_curve_.emplace(curve);
   for (auto& observer : observers_)
     observer.OnModelTrained(curve);
 

chrome/browser/chromeos/power/auto_screen_brightness/modeller_impl.h

@@ -188,12 +188,18 @@ class ModellerImpl : public Modeller,
 
   base::Optional<AlsReader::AlsInitStatus> als_init_status_;
   base::Optional<bool> brightness_monitor_success_;
-  Status model_status_ = Status::kInitializing;
+
+  // Whether this modeller has initialized successfully, including connecting
+  // to AlsReader, BrightnessMonitor and loading a Trainer.
+  // Initially has no value. Guaranteed to have a value after the completion of
+  // |OnCurveLoadedFromDisk|.
+  base::Optional<bool> is_modeller_enabled_;
 
   base::FilePath curve_path_;
 
-  // Latest personal curve, either loaded from disk or trained.
-  base::Optional<MonotoneCubicSpline> personal_curve_;
+  // True if a personal curve was successfully loaded from disk and passed to
+  // Trainer and Trainer reported it was valid.
+  bool has_initial_personal_curve_ = false;
 
   // Global curve constructed from predefined params.
   const MonotoneCubicSpline global_curve_;

chrome/browser/chromeos/power/auto_screen_brightness/trainer.h

@@ -20,13 +20,37 @@ struct TrainingDataPoint {
 };
 
 // Interface to train an on-device adaptive brightness curve.
+// User should call |HasValidConfiguration| first. If it returns true, then user
+// should call |SetInitialCurves| before calling other methods.
 class Trainer {
  public:
   virtual ~Trainer() = default;
 
-  virtual void SetInitialCurves(const MonotoneCubicSpline& global_curve,
+  // Returns whether trainer has been configured properly, i.e. if all params
+  // are set up. It is an error to call other methods unless
+  // |HasValidConfiguration| returns true.
+  virtual bool HasValidConfiguration() const = 0;
+
+  // Initializes this trainer with the specified default global curve and
+  // initial current curve (the personal curve). This should only be called if
+  // trainer |HasValidConfiguration| returns true.
+  // Returns true if |current_curve| is valid, i.e. satisfying constraints (e.g.
+  // slope). If |current_curve| is invalid, |global_curve| will be used in its
+  // place. The caller has an option to reset these curves.
+  virtual bool SetInitialCurves(const MonotoneCubicSpline& global_curve,
                                 const MonotoneCubicSpline& current_curve) = 0;
 
+  // Returns the global curve. This should only be called if trainer
+  // |HasValidConfiguration| returns true and after |SetInitialCurves| is
+  // called.
+  virtual MonotoneCubicSpline GetGlobalCurve() const = 0;
+
+  // Returns the curve currently used as personal curve. It could be the same as
+  // the global curve. This should only be called if trainer
+  // |HasValidConfiguration| returns true and after |SetInitialCurves| is
+  // called.
+  virtual MonotoneCubicSpline GetCurrentCurve() const = 0;
+
   // Updates current curve stored in trainer with |data|. This function should
   // only be called after |SetInitialCurves|.
   virtual MonotoneCubicSpline Train(