[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4250: SoC - Separate out Tuning from Translator

[GitBox <gi...@apache.org>]

jihaozh commented on a change in pull request #4250: SoC - Separate out Tuning from Translator
URL: https://github.com/apache/incubator-pinot/pull/4250#discussion_r288683060
 
 

 ##########
 File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java
 ##########
 @@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pinot.thirdeye.detection.yaml;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import org.apache.commons.collections.MapUtils;
+import org.apache.pinot.thirdeye.datalayer.dto.DatasetConfigDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.DetectionConfigDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.MetricConfigDTO;
+import org.apache.pinot.thirdeye.detection.ConfigUtils;
+import org.apache.pinot.thirdeye.detection.DataProvider;
+import org.apache.pinot.thirdeye.detection.DefaultInputDataFetcher;
+import org.apache.pinot.thirdeye.detection.DetectionUtils;
+import org.apache.pinot.thirdeye.detection.InputDataFetcher;
+import org.apache.pinot.thirdeye.detection.annotation.registry.DetectionRegistry;
+import org.apache.pinot.thirdeye.detection.spec.AbstractSpec;
+import org.apache.pinot.thirdeye.detection.spi.components.Tunable;
+import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+import org.joda.time.Interval;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.thirdeye.detection.DetectionUtils.*;
+
+public class DetectionConfigTuner {
+  protected static final Logger LOG = LoggerFactory.getLogger(DetectionConfigTuner.class);
+
+  private static final String PROP_FILTERS = "filters";
+  private static final String PROP_CLASS_NAME = "className";
+  private static final String DEFAULT_TIMEZONE = "America/Los_Angeles";
+  private static final String PROP_METRIC = "metric";
+  private static final String PROP_DATASET = "dataset";
+
+  private static final DetectionRegistry DETECTION_REGISTRY = DetectionRegistry.getInstance();
+  static {
+    // do not tune for alerts migrated from legacy anomaly function.
+    DetectionRegistry.registerComponent("com.linkedin.thirdeye.detection.components.AdLibAlertFilter",
+        "MIGRATED_ALGORITHM_FILTER");
+    DetectionRegistry.registerComponent("com.linkedin.thirdeye.detection.components.AdLibAnomalyDetector",
+        "MIGRATED_ALGORITHM");
+  }
+
+  public static final String PROP_YAML_PARAMS = "yamlParams";
+  public static final Set<String> TURNOFF_TUNING_COMPONENTS =
+      ImmutableSet.of("MIGRATED_ALGORITHM_FILTER", "MIGRATED_ALGORITHM", "MIGRATED_ALGORITHM_BASELINE");
+
+  private final DetectionConfigDTO detectionConfig;
+  private final DataProvider dataProvider;
+  private final DatasetConfigDTO datasetConfig;
+  private final String metricUrn;
+
+  public DetectionConfigTuner(DetectionConfigDTO config, DataProvider dataProvider) {
+    this.detectionConfig = config;
+    this.dataProvider = dataProvider;
+
+    Map<String, Object> yamlConfig = ConfigUtils.getMap(new org.yaml.snakeyaml.Yaml().load(config.getYaml()));
+
+    MetricConfigDTO metricConfig = dataProvider.fetchMetric(
+        MapUtils.getString(yamlConfig, PROP_METRIC),
+        MapUtils.getString(yamlConfig, PROP_DATASET));
+    Preconditions.checkNotNull(metricConfig, "metric not found");
+    this.datasetConfig = dataProvider.fetchDatasets(Collections.singletonList(metricConfig.getDataset()))
+        .get(metricConfig.getDataset());
+    Preconditions.checkNotNull(this.datasetConfig, "dataset not found");
+
+    this.metricUrn = MetricEntity.buildMetricUrn(MapUtils.getMap(yamlConfig, PROP_FILTERS), metricConfig.getId());
+  }
+
+  /**
+   * Returns the default(for new alert) or tuned configuration for the specified component
+   *
+   * @param componentKey the key used to reference the component within the detection
+   * @param componentProps previously tuned configuration along with user supplied yaml config
+   * @param startTime start time of the tuning window
+   * @param endTime end time of the tuning window
+   */
+  private Map<String, Object> getTunedSpecs(String componentKey, Map<String, Object> componentProps, long startTime, long endTime)
+      throws Exception {
+    // Instantiate tunable component
+    long configId = detectionConfig == null ? 0 : detectionConfig.getId();
+    String componentClassName = componentProps.get(PROP_CLASS_NAME).toString();
+    Map<String, Object> yamlParams = ConfigUtils.getMap(componentProps.get(PROP_YAML_PARAMS));
+    InputDataFetcher dataFetcher = new DefaultInputDataFetcher(dataProvider, configId);
+    Tunable tunable = instantiateTunable(componentClassName, yamlParams, dataFetcher);
+
+    // round to daily boundary
+    DateTimeZone timezone = DateTimeZone.forID(this.datasetConfig.getTimezone() == null ? DEFAULT_TIMEZONE : this.datasetConfig.getTimezone());
+    DateTime start = new DateTime(startTime, timezone).withTimeAtStartOfDay();
+    DateTime end =  new DateTime(endTime, timezone).withTimeAtStartOfDay();
+    Interval window = new Interval(start, end);
+
+    // TODO can you use componentProps instead? what is the difference btw component and componentSpec
+    Map<String, Object> existingComponentSpec =
+        detectionConfig.getComponents().containsKey(componentKey) ?
+            MapUtils.getMap(detectionConfig.getComponents(), componentKey) : Collections.emptyMap();
+
+    // TODO: if dimension drill down applied, pass in the metric urn of top dimension
+    return tunable.tune(existingComponentSpec, window, this.metricUrn);
+  }
+
+  private Tunable instantiateTunable(String componentClassName, Map<String, Object> yamlParams, InputDataFetcher dataFetcher)
+      throws Exception {
+    String tunableClassName = DETECTION_REGISTRY.lookupTunable(componentClassName);
+    Class clazz = Class.forName(tunableClassName);
+    Class<AbstractSpec> specClazz = (Class<AbstractSpec>) Class.forName(getSpecClassName(clazz));
+    AbstractSpec spec = AbstractSpec.fromProperties(yamlParams, specClazz);
+    Tunable tunable = (Tunable) clazz.newInstance();
+    tunable.init(spec, dataFetcher);
+    return tunable;
+  }
+
+  /**
+   * Scans through all the tunable components and injects tuned model params into the detection config
+   */
+  public DetectionConfigDTO tune(long tuningWindowStart, long tuningWindowEnd) {
+    Map<String, Object> allComponentSpecs = this.detectionConfig.getComponentSpecs();
+    for (Map.Entry<String, Object> componentSpec : allComponentSpecs.entrySet()) {
+      String componentKey = componentSpec.getKey();
+      Map<String, Object> componentProps = ConfigUtils.getMap(componentSpec.getValue());
+
+      // For tunable components, the model params are computed from user supplied yaml params and previous model params.
+      String componentClassName = componentProps.get(PROP_CLASS_NAME).toString();
+      String type = DetectionUtils.getComponentType(componentKey);
+      if (!TURNOFF_TUNING_COMPONENTS.contains(type) && DETECTION_REGISTRY.isTunable(componentClassName)) {
+        try {
+          componentProps.putAll(getTunedSpecs(componentKey, componentProps, tuningWindowStart, tuningWindowEnd));
+          detectionConfig.setComponentSpecs(componentProps);
 
 Review comment:
   is `componentProps` the properties for one component? should we set it as  `setComponentSpecs` of the whole detection config?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org