Fix issues with LayeredModelConfig accessing underlying model attribu…

…tes. (#104)

* Simplify BOCPD implementation.

* Remove overcomplicated _online_model check.

* Fix LayeredModel access to underlying model params

* Add docstring.

* Have AutoMLMixin inherit from LayeredModel.

* Add feature summary line for change point detect.

* Update version to 1.2.1.

README.md

@@ -62,6 +62,7 @@ anomaly detection and/or forecasting.
 | Multivariate Forecasting | ✅ | | | ✅ | ✅ | ✅ | | | | |
 | Univariate Anomaly Detection | ✅ | ✅ | ✅ | ✅ | | | ✅ | ✅ | ✅ | ✅ | 
 | Multivariate Anomaly Detection | ✅ | | ✅ | ✅ | | | ✅ | ✅ | | | |
+| Change Point Detection | ✅ | ✅ | ✅ | ✅ | | | | | ✅ | |
 | AutoML | ✅ | | | ✅ | | | | | ✅ | | ✅ 
 | Ensembles | ✅ | | | | | | ✅  | | | | 
 | Benchmarking | ✅ | | | | | ✅ | | | | | 

merlion/models/anomaly/change_point/bocpd.py

@@ -11,7 +11,7 @@
 import copy
 from enum import Enum
 import logging
-from typing import Iterator, List, Tuple, Union
+from typing import List, Tuple, Union
 import warnings
 
 import numpy as np
@@ -21,7 +21,6 @@
 from scipy.stats import norm
 from tqdm import tqdm
 
-from merlion.models.automl.base import AutoMLMixIn, ModelBase
 from merlion.models.anomaly.base import NoCalibrationDetectorConfig, DetectorBase
 from merlion.models.anomaly.forecast_based.base import ForecastingDetectorBase
 from merlion.models.forecast.base import ForecasterConfig
@@ -136,7 +135,7 @@ def change_kind(self, change_kind: Union[str, ChangeKind]):
         self._change_kind = change_kind
 
 
-class BOCPD(AutoMLMixIn, ForecastingDetectorBase):
+class BOCPD(ForecastingDetectorBase):
     """
     Bayesian online change point detection algorithm described by
     `Adams & MacKay (2007) <https://arxiv.org/abs/0710.3742>`__.
@@ -216,50 +215,6 @@ def min_likelihood(self) -> float:
         """
         return self.config.min_likelihood
 
-    @property
-    def model(self):
-        """
-        :return: The model itself. Implemented to support the ``train()`` method of `AutoMLMixIn`.
-            The setter is equivalent to calling ``self.__setstate__(model.__getstate__())``.
-        """
-        return self
-
-    @model.setter
-    def model(self, model):
-        self.__setstate__(model.__getstate__())
-
-    def generate_theta(self, train_data: TimeSeries) -> Iterator:
-        if self.change_kind is not ChangeKind.Auto:
-            return iter([self.change_kind])
-        else:
-            return iter(ck for ck in ChangeKind if ck is not ChangeKind.Auto)
-
-    def set_theta(self, model, theta, train_data: TimeSeries = None):
-        model.config.change_kind = theta
-
-    def evaluate_theta(
-        self, thetas: Iterator, train_data: TimeSeries, train_config=None, **kwargs
-    ) -> Tuple[float, ModelBase, TimeSeries]:
-        # If not automatically detecting the change kind, train as normal
-        if self.change_kind is not ChangeKind.Auto:
-            train_scores = ForecastingDetectorBase.train(self, train_data, train_config=train_config, **kwargs)
-            log_likelihood = logsumexp([p.logp for p in self.posterior_beam]).item()
-            return log_likelihood, self, train_scores
-
-        # Otherwise, evaluate all thetas as options
-        candidates = []
-        for change_kind in thetas:
-            candidate = copy.deepcopy(self)
-            candidate.config.change_kind = change_kind
-            train_scores = candidate.train(train_data, train_config=train_config, **kwargs)
-            log_likelihood = logsumexp([p.logp for p in candidate.posterior_beam]).item()
-            candidates.append((log_likelihood, candidate, train_scores))
-            logger.info(f"Change kind {change_kind.name} has log likelihood {log_likelihood:.3f}.")
-
-        # Choose the model with the best log likelihood
-        i_best = np.argmax([candidate[0] for candidate in candidates])
-        return candidates[i_best]
-
     def _create_posterior(self, logp: float) -> _PosteriorBeam:
         posterior = self.change_kind.value()
         return _PosteriorBeam(run_length=0, posterior=posterior, cp_prior=self.cp_prior, logp=logp)
@@ -460,7 +415,26 @@ def update(self, time_series: TimeSeries):
         return self._get_anom_scores(time_stamps)
 
     def _train(self, train_data: pd.DataFrame, train_config=None) -> pd.DataFrame:
-        return self.update(time_series=TimeSeries.from_pd(train_data)).to_pd()
+        # If not automatically detecting the change kind, train as normal
+        if self.change_kind is not ChangeKind.Auto:
+            return self.update(time_series=TimeSeries.from_pd(train_data)).to_pd()
+
+        # Otherwise, evaluate all change kinds as options
+        candidates = []
+        for change_kind in [ck for ck in ChangeKind if ck is not ChangeKind.Auto]:
+            candidate = copy.deepcopy(self)
+            candidate.config.change_kind = change_kind
+            train_scores = candidate._train(train_data, train_config=train_config)
+            log_likelihood = logsumexp([p.logp for p in candidate.posterior_beam]).item()
+            candidates.append((log_likelihood, candidate, train_scores))
+            logger.info(f"Change kind {change_kind.name} has log likelihood {log_likelihood:.3f}.")
+
+        # Choose the model with the best log likelihood
+        i_best = np.argmax([candidate[0] for candidate in candidates])
+        log_likelihood, best, train_scores = candidates[i_best]
+        self.__setstate__(best.__getstate__())
+        logger.info(f"Using change kind {self.change_kind.name} because it has the best log likelihood.")
+        return train_scores
 
     def get_anomaly_score(self, time_series: TimeSeries, time_series_prev: TimeSeries = None) -> TimeSeries:
         return DetectorBase.get_anomaly_score(self, time_series, time_series_prev)

merlion/models/automl/base.py

@@ -11,22 +11,22 @@
 from copy import deepcopy
 from typing import Any, Iterator, Optional, Tuple
 
-from merlion.models.base import ModelBase
+from merlion.models.layers import ModelBase, LayeredModel
 from merlion.utils import TimeSeries
 from merlion.utils.misc import AutodocABCMeta
 
 
-class AutoMLMixIn(ModelBase, metaclass=AutodocABCMeta):
+class AutoMLMixIn(LayeredModel, metaclass=AutodocABCMeta):
     """
-    Base Interface for Implemented AutoML Layers
-
-    This abstract class contains all of the methods that Layers should implement. Ideally, these would be generated by
-    an existing mix-in.
+    Abstract base class which converts `LayeredModel` into an AutoML models.
     """
 
-    def train(self, train_data: TimeSeries, **kwargs):
-        train_data = self.train_pre_process(train_data)
+    def train_model(self, train_data: TimeSeries, **kwargs):
+        """
+        Generates a set of candidate models and picks the best one.
 
+        :param train_data: the data to train on, after any pre-processing transforms have been applied.
+        """
         candidate_thetas = self.generate_theta(train_data)
         theta, model, train_result = self.evaluate_theta(candidate_thetas, train_data, kwargs)
         if model is not None:

merlion/models/automl/seasonality.py

@@ -14,7 +14,7 @@
 
 from merlion.models.automl.base import AutoMLMixIn
 from merlion.models.base import ModelBase
-from merlion.models.layers import LayeredModelConfig, LayeredModel
+from merlion.models.layers import LayeredModelConfig
 from merlion.transform.resample import TemporalResample
 from merlion.utils import TimeSeries, UnivariateTimeSeries, autosarima_utils
 from merlion.utils.misc import AutodocABCMeta
@@ -117,7 +117,7 @@ def to_dict(self, _skipped_keys=None):
         return config_dict
 
 
-class SeasonalityLayer(AutoMLMixIn, LayeredModel, metaclass=AutodocABCMeta):
+class SeasonalityLayer(AutoMLMixIn, metaclass=AutodocABCMeta):
     """
     Seasonality Layer that uses AutoSARIMA-like methods to determine seasonality of your data. Can be used directly on
     any model that implements `SeasonalityModel` class.

merlion/models/forecast/base.py

@@ -30,6 +30,7 @@ class ForecasterConfig(Config):
 
     max_forecast_steps: Optional[int] = None
     target_seq_index: Optional[int] = None
+    invert_transform: bool = False
 
     def __init__(self, max_forecast_steps: int = None, target_seq_index: int = None, invert_transform=False, **kwargs):
         """
@@ -89,10 +90,6 @@ def invert_transform(self):
         """
         return self.config.invert_transform
 
-    @property
-    def _online_model(self) -> bool:
-        return False
-
     @property
     def require_univariate(self) -> bool:
         """
@@ -234,17 +231,12 @@ def forecast(
         if time_series_prev is None:
             time_series_prev_df = None
         else:
-            tf = to_pd_datetime(self.last_train_time + self.timedelta)
             time_series_prev = self.transform(time_series_prev)
             assert time_series_prev.dim == self.dim, (
                 f"time_series_prev has dimension of {time_series_prev.dim} that is different from "
                 f"training data dimension of {self.dim} for the model"
             )
-            if self._online_model and to_pd_datetime(time_series_prev.tf) < tf:
-                time_series_prev = None
-                time_series_prev_df = None
-            else:
-                time_series_prev_df = time_series_prev.to_pd()
+            time_series_prev_df = time_series_prev.to_pd()
 
         # Make the prediction
         forecast, err = self._forecast(

merlion/models/forecast/ets.py

@@ -108,10 +108,6 @@ def seasonal(self):
     def seasonal_periods(self):
         return self.config.seasonal_periods
 
-    @property
-    def _online_model(self) -> bool:
-        return True
-
     def set_seasonality(self, theta, train_data: UnivariateTimeSeries):
         if theta > 1:
             self.config.seasonal_periods = int(theta)
@@ -154,6 +150,9 @@ def _train(self, train_data: pd.DataFrame, train_config=None):
     def _forecast(
         self, time_stamps: Union[int, List[int]], time_series_prev: pd.DataFrame = None, return_prev=False
     ) -> Tuple[pd.DataFrame, pd.DataFrame]:
+        # Ignore time_series_prev if it comes after the training data
+        if time_series_prev is not None and time_series_prev.index[-1] < self.last_train_time + self.timedelta:
+            time_series_prev = None
 
         # Basic forecasting without time_series_prev
         if time_series_prev is None:

merlion/models/layers.py

@@ -10,6 +10,7 @@
 `AutoML models <merlion.models.automl>`_.
 """
 import copy
+import inspect
 import logging
 from typing import Any, Dict, List, Union
 
@@ -20,11 +21,16 @@
 from merlion.models.anomaly.base import DetectorBase, DetectorConfig
 from merlion.models.forecast.base import ForecasterBase, ForecasterConfig
 from merlion.models.anomaly.forecast_based.base import ForecastingDetectorBase
+from merlion.transform.base import Identity
+from merlion.transform.sequence import TransformSequence
 from merlion.utils import TimeSeries
 from merlion.utils.misc import AutodocABCMeta
 
 logger = logging.getLogger(__name__)
 
+_DETECTOR_MEMBERS = dict(inspect.getmembers(DetectorConfig)).keys()
+_FORECASTER_MEMBERS = dict(inspect.getmembers(ForecasterConfig)).keys()
+
 
 class LayeredModelConfig(Config):
     """
@@ -43,7 +49,13 @@ def __init__(self, model: Union[ModelBase, Dict], model_kwargs=None, **kwargs):
         # Model-specific kwargs override kwargs when creating the model.
         model_kwargs = {} if model_kwargs is None else model_kwargs
         if isinstance(model, dict):
-            model.update({k: v for k, v in kwargs.items() if k not in model and k not in model_kwargs})
+            model.update(
+                {
+                    k: v
+                    for k, v in kwargs.items()
+                    if k not in model and k not in model_kwargs and k not in _LAYERED_MEMBERS
+                }
+            )
             model, extra_kwargs = ModelFactory.create(**{**model, **model_kwargs, "return_unused_kwargs": True})
             kwargs.update(extra_kwargs)
         self.model = model
@@ -82,7 +94,7 @@ def to_dict(self, _skipped_keys=None):
     def from_dict(cls, config_dict: Dict[str, Any], return_unused_kwargs=False, dim=None, **kwargs):
         config, kwargs = super().from_dict(config_dict=config_dict, return_unused_kwargs=True, dim=dim, **kwargs)
         if config.model is None:
-            used = {k: v for k, v in kwargs.items() if hasattr(DetectorConfig, k) or hasattr(ForecasterConfig, k)}
+            used = {k: v for k, v in kwargs.items() if k in _DETECTOR_MEMBERS or k in _FORECASTER_MEMBERS}
             config.model_kwargs.update(used)
             kwargs = {k: v for k, v in kwargs.items() if k not in used}
 
@@ -106,8 +118,8 @@ def __getattr__(self, item):
         if item in ["model", "_model", "base_model"]:
             return super().__getattribute__(item)
         base_model = self.base_model
-        is_detector_attr = isinstance(base_model, DetectorBase) and hasattr(DetectorConfig, item)
-        is_forecaster_attr = isinstance(base_model, ForecasterBase) and hasattr(ForecasterConfig, item)
+        is_detector_attr = isinstance(base_model, DetectorBase) and item in _DETECTOR_MEMBERS
+        is_forecaster_attr = isinstance(base_model, ForecasterBase) and item in _FORECASTER_MEMBERS
         if is_detector_attr or is_forecaster_attr:
             return getattr(base_model.config, item)
         elif base_model is None and item in self.model_kwargs:
@@ -117,10 +129,9 @@ def __getattr__(self, item):
     def __setattr__(self, key, value):
         if hasattr(self, "_model"):
             base_model = self.base_model
-            is_detector_attr = isinstance(base_model, DetectorBase) and hasattr(DetectorConfig, key)
-            is_forecaster_attr = isinstance(base_model, ForecasterBase) and hasattr(ForecasterConfig, key)
-            is_layered_attr = hasattr(LayeredModelConfig, key)
-            if not is_layered_attr and (is_detector_attr or is_forecaster_attr):
+            is_detector_attr = isinstance(base_model, DetectorBase) and key in _DETECTOR_MEMBERS
+            is_forecaster_attr = isinstance(base_model, ForecasterBase) and key in _FORECASTER_MEMBERS
+            if key not in _LAYERED_MEMBERS and (is_detector_attr or is_forecaster_attr):
                 return setattr(self.model.config, key, value)
         return super().__setattr__(key, value)
 
@@ -133,6 +144,9 @@ def get_unused_kwargs(self, **kwargs):
         return {k: v for k, v in kwargs.items() if k not in valid_keys}
 
 
+_LAYERED_MEMBERS = dict(inspect.getmembers(LayeredModelConfig)).keys()
+
+
 class LayeredModel(ModelBase, metaclass=AutodocABCMeta):
     """
     Abstract class implementing a model which wraps around another internal model.
@@ -158,7 +172,9 @@ class LayeredModel(ModelBase, metaclass=AutodocABCMeta):
 
     .. note::
 
-        For the time being, every layer of the model is allowed to have its own ``transform``.
+        For the time being, every layer of the model is allowed to have its own ``transform``. However, after the
+        model is trained, the entire transform will be composed as a single `TransformSequence` and will be owned by
+        the base model.
     """
 
     config_class = LayeredModelConfig
@@ -272,9 +288,21 @@ def __getattr__(self, item):
             return attr
         return self.__getattribute__(item)
 
+    def train_model(self, train_data, *args, **kwargs):
+        """
+        Trains the underlying model. May be overridden, e.g. for AutoML.
+
+        :param train_data: the data to train on, after any pre-processing transforms have been applied.
+        """
+        return self.model.train(train_data, *args, **kwargs)
+
     def train(self, train_data: TimeSeries, *args, **kwargs):
         train_data = self.train_pre_process(train_data)
-        return self.model.train(train_data, *args, **kwargs)
+        ret = self.train_model(train_data, *args, **kwargs)
+        # Push the layered model transform to the owned model
+        self.model.transform = TransformSequence([self.transform, self.model.transform])
+        self.transform = Identity()
+        return ret
 
 
 class LayeredDetector(LayeredModel, DetectorBase):
@@ -289,7 +317,6 @@ def _get_anomaly_score(self, time_series: pd.DataFrame, time_series_prev: pd.Dat
         raise NotImplementedError("Layered model _get_anomaly_score() should not be called.")
 
     def get_anomaly_score(self, time_series: TimeSeries, time_series_prev: TimeSeries = None) -> TimeSeries:
-        time_series, time_series_prev = self.transform_time_series(time_series, time_series_prev)
         return self.model.get_anomaly_score(time_series, time_series_prev)
 
 
@@ -305,8 +332,6 @@ def _forecast(self, time_stamps: List[int], time_series_prev: TimeSeries = None,
         raise NotImplementedError("Layered model _forecast() should not be called.")
 
     def forecast(self, time_stamps, time_series_prev: TimeSeries = None, *args, **kwargs):
-        if time_series_prev is not None:
-            time_series_prev = self.transform(time_series_prev)
         return self.model.forecast(time_stamps, time_series_prev, *args, **kwargs)
 
 

setup.py

@@ -28,7 +28,7 @@ def read_file(fname):
 
 setup(
     name="salesforce-merlion",
-    version="1.2.0",
+    version="1.2.1",
     author=", ".join(read_file("AUTHORS.md").split("\n")),
     author_email="[email protected]",
     description="Merlion: A Machine Learning Framework for Time Series Intelligence",