Repository: dssg/triage

Issue:
Add more feature subsetting strategies
According to @rayidghani We can't use leave-one-out without leave-one-in, so we should fill out the strategies now.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=h1) Report
> Merging [#89](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=desc) into [master](https://codecov.io/gh/dssg/triage/commit/259af31519000bc2bc94f9300eaf4bc596631551?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `88.88%`.

[![Impacted file tree graph](https://codecov.io/gh/dssg/triage/pull/89/graphs/tree.svg?width=650&src=pr&height=150&token=SxkQCokhnJ)](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
- Coverage   89.74%   89.72%   -0.02%     
==========================================
  Files          22       22              
  Lines         946      954       +8     
==========================================
+ Hits          849      856       +7     
- Misses         97       98       +1
```


| [Impacted Files](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [triage/feature\_group\_creator.py](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=tree#diff-dHJpYWdlL2ZlYXR1cmVfZ3JvdXBfY3JlYXRvci5weQ==) | `92.85% <ø> (ø)` | :arrow_up: |
| [triage/feature\_group\_mixer.py](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=tree#diff-dHJpYWdlL2ZlYXR1cmVfZ3JvdXBfbWl4ZXIucHk=) | `96.55% <88.88%> (-3.45%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=footer). Last update [259af31...fdd0c66](https://codecov.io/gh/dssg/triage/pull/89?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/example_experiment_config.yaml b/example_experiment_config.yaml
index 2b5cd470..5eb574d3 100644
--- a/example_experiment_config.yaml
+++ b/example_experiment_config.yaml
@@ -98,7 +98,7 @@ feature_group_definition:
     tables: ['prefix_entity_id']
 
 # strategies for generating combinations of groups
-# available: all, leave-one-out
+# available: all, leave-one-out, leave-one-in
 feature_group_strategies: ['all']
 
 
diff --git a/triage/db.py b/triage/db.py
index 855a992c..734ef978 100644
--- a/triage/db.py
+++ b/triage/db.py
@@ -52,8 +52,8 @@ class ModelGroup(Base):
     model_group_id = Column(Integer, primary_key=True)
     model_type = Column(Text)
     model_parameters = Column(JSONB)
-    prediction_window = Column(Text)
     feature_list = Column(ARRAY(Text))
+    model_config = Column(JSONB)
 
 
 class Model(Base):
diff --git a/triage/feature_group_creator.py b/triage/feature_group_creator.py
index 95c82033..0c60dfc5 100644
--- a/triage/feature_group_creator.py
+++ b/triage/feature_group_creator.py
@@ -20,6 +20,7 @@ def all_subsetter(config_item, table, features):
 
 
 class FeatureGroupCreator(object):
+    """Divides a feature dictionary into groups based on given criteria"""
     subsetters = {
         'tables': table_subsetter,
         'prefix': prefix_subsetter,
@@ -45,7 +46,15 @@ class FeatureGroupCreator(object):
         Args:
             feature_dictionary (dict) tables and the features contained in each
 
-        Returns: (list) subsets of the feature dictionary
+            The feature dictionary is meant to be keyed on source table. Example:
+
+            {
+                'feature_table_one': ['feature_one', feature_two'],
+                'feature_table_two': ['feature_three', 'feature_four'],
+            }
+
+        Returns: (list) subsets of the feature dictionary, in the same
+            table-based structure
         """
         subsets = []
         for name, config in sorted(self.definition.items()):
diff --git a/triage/feature_group_mixer.py b/triage/feature_group_mixer.py
index a04fa900..ff8f6cfe 100644
--- a/triage/feature_group_mixer.py
+++ b/triage/feature_group_mixer.py
@@ -1,3 +1,14 @@
+def leave_one_in(feature_groups):
+    """For each group, return a copy of just that group
+
+    Args:
+        feature_groups (list) The feature groups to apply the strategy to
+
+    Returns: A list of feature dicts
+    """
+    return feature_groups
+
+
 def leave_one_out(feature_groups):
     """For each group, return a copy of all groups excluding that group
 
@@ -18,7 +29,17 @@ def leave_one_out(feature_groups):
 
 
 def all_features(feature_groups):
-    return feature_groups
+    """Return a combination of all feature groups
+
+    Args:
+        feature_groups (list) The feature groups to apply the strategy to
+
+    Returns: A list of feature dicts
+    """
+    feature_dict = {}
+    for group in feature_groups:
+        feature_dict.update(group)
+    return [feature_dict]
 
 
 class FeatureGroupMixer(object):
@@ -26,10 +47,14 @@ class FeatureGroupMixer(object):
     based on a list of strategies"""
     strategy_lookup = {
         'leave-one-out': leave_one_out,
+        'leave-one-in': leave_one_in,
         'all': all_features,
     }
 
     def __init__(self, strategies):
+        for strategy in strategies:
+            if strategy not in self.strategy_lookup:
+                raise ValueError('Unknown strategy "{}"'.format(strategy))
         self.strategies = strategies
 
     def generate(self, feature_groups):
diff --git a/triage/model_group_stored_procedure.sql b/triage/model_group_stored_procedure.sql
index 84a6d9b7..8d3b2a5a 100644
--- a/triage/model_group_stored_procedure.sql
+++ b/triage/model_group_stored_procedure.sql
@@ -12,9 +12,9 @@ CREATE TABLE results.model_groups
 -----------
 populates the table and returns the IDs
 */
-CREATE OR REPLACE FUNCTION get_model_group_id(in_model_type        TEXT, in_model_parameters JSONB,
-                                              in_prediction_window TEXT,
-                                              in_feature_list      TEXT [])
+CREATE OR REPLACE FUNCTION public.get_model_group_id(in_model_type        TEXT, in_model_parameters JSONB,
+                                              in_feature_list      TEXT [],
+                                              in_model_config      JSONB)
   RETURNS INTEGER AS
 $BODY$
 DECLARE
@@ -28,12 +28,14 @@ BEGIN
   INTO model_group_return_id
   FROM results.model_groups
   WHERE
-    model_type = in_model_type AND model_parameters = in_model_parameters AND prediction_window = in_prediction_window
-    AND feature_list = ARRAY(Select unnest(in_feature_list) ORDER BY 1);
+    model_type = in_model_type 
+    AND model_parameters = in_model_parameters 
+    AND feature_list = ARRAY(Select unnest(in_feature_list) ORDER BY 1)
+    AND model_config = in_model_config ;
   IF NOT FOUND
   THEN
-    INSERT INTO results.model_groups (model_group_id, model_type, model_parameters, prediction_window, feature_list)
-    VALUES (DEFAULT, in_model_type, in_model_parameters, in_prediction_window, ARRAY(Select unnest(in_feature_list) ORDER BY 1))
+    INSERT INTO results.model_groups (model_group_id, model_type, model_parameters, feature_list, model_config)
+    VALUES (DEFAULT, in_model_type, in_model_parameters, ARRAY(Select unnest(in_feature_list) ORDER BY 1), in_model_config)
     RETURNING model_group_id
       INTO model_group_return_id;
   END IF;
diff --git a/triage/model_trainers.py b/triage/model_trainers.py
index 38d9bd18..6da11474 100644
--- a/triage/model_trainers.py
+++ b/triage/model_trainers.py
@@ -10,7 +10,6 @@ import pandas
 import warnings
 from triage.utils import filename_friendly_hash
 
-
 def get_feature_importances(model):
     """
     Get feature importances (from scikit-learn) of trained model.
@@ -168,10 +167,10 @@ class ModelTrainer(object):
             rankings_pct):
             feature_importance = FeatureImportance(
                 model=model,
-                feature_importance=importance,
+                feature_importance=round(float(importance), 10),
                 feature=feature_names[feature_index],
                 rank_abs=int(rank_abs),
-                rank_pct=float(rank_pct)
+                rank_pct=round(float(rank_pct), 10)
             )
             session.add(feature_importance)
         session.commit()
@@ -207,8 +206,8 @@ class ModelTrainer(object):
         model_group_id = self._get_model_group_id(
              class_path,
              parameters,
-             matrix_store.metadata['prediction_window'],
-             matrix_store.metadata['feature_names']
+             matrix_store.metadata['feature_names'],
+             matrix_store.metadata.get('model_config', dict())
         )
         logging.debug('Trained model')
         model_store.write(trained_model)
@@ -229,8 +228,8 @@ class ModelTrainer(object):
         self,
         class_path,
         parameters,
-        prediction_window,
-        feature_names
+        feature_names,
+        model_config
     ):
         """
         Returns model group id using store procedure 'get_model_group_id' which will 
@@ -240,9 +239,8 @@ class ModelTrainer(object):
         Args:
             class_path (string) A full classpath to the model class
             parameters (dict) hyperparameters to give to the model constructor
-            prediction_window (string) The prediction window used for generating the labels
-                                       stored in metadata
-           features_names (list) Features used for train/test
+            features_names (list) Features used for train/test
+            model_config (dict) Dictionary of the classes for comparing groups
 
         Returns: (int) a database id for the model group id
         """
@@ -259,12 +257,13 @@ class ModelTrainer(object):
             query = ("SELECT get_model_group_id( "
                      "            '{class_path}'::TEXT, "
                      "            '{parameters}'::JSONB, "
-                     "            '{prediction_window}'::TEXT, "
-                     "             ARRAY{feature_names}::TEXT [] )"
+                     "             ARRAY{feature_names}::TEXT [] , "
+                     "            '{model_config}'::JSONB )"
                      .format(class_path=class_path,
                              parameters=json.dumps(parameters),
-                             prediction_window=prediction_window,
-                             feature_names=feature_names))
+                             feature_names=feature_names,
+                             model_config=json.dumps(model_config, sort_keys=True)))
+
             cur.execute(query)
             db_conn.commit()
             model_group_id = cur.fetchone()
diff --git a/triage/predictors.py b/triage/predictors.py
index 26414da7..825e8861 100644
--- a/triage/predictors.py
+++ b/triage/predictors.py
@@ -113,10 +113,10 @@ class Predictor(object):
                     model_id=int(model_id),
                     entity_id=int(entity_id),
                     as_of_date=matrix_end_time,
-                    score=float(score),
+                    score=round(float(score), 10),
                     label_value=int(label) if not math.isnan(label) else None,
                     rank_abs=int(rank_abs),
-                    rank_pct=float(rank_pct),
+                    rank_pct=round(float(rank_pct),10),
                     **misc_db_parameters
                 ))
 


Test patch:
diff --git a/tests/test_feature_group_mixer.py b/tests/test_feature_group_mixer.py
index 2edeb1bb..a369388e 100644
--- a/tests/test_feature_group_mixer.py
+++ b/tests/test_feature_group_mixer.py
@@ -16,3 +16,31 @@ def test_feature_group_mixer_leave_one_out():
         dict(itertools.chain(english_numbers.items(), letters.items())),
     ]
     assert result == expected
+
+
+def test_feature_group_mixer_leave_one_in():
+    english_numbers = {'one': ['two', 'three'], 'four': ['five', 'six']}
+    letters = {'a': ['b', 'c'], 'd': ['e', 'f']}
+    german_numbers = {'eins': ['zwei', 'drei'], 'vier': ['funf', 'sechs']}
+    feature_groups = [english_numbers, letters, german_numbers]
+
+    result = FeatureGroupMixer(['leave-one-in']).generate(feature_groups)
+    expected = [
+        english_numbers,
+        letters,
+        german_numbers
+    ]
+    assert result == expected
+
+
+def test_feature_group_mixer_all():
+    english_numbers = {'one': ['two', 'three'], 'four': ['five', 'six']}
+    letters = {'a': ['b', 'c'], 'd': ['e', 'f']}
+    german_numbers = {'eins': ['zwei', 'drei'], 'vier': ['funf', 'sechs']}
+    feature_groups = [english_numbers, letters, german_numbers]
+
+    result = FeatureGroupMixer(['all']).generate(feature_groups)
+    expected = [
+        dict(itertools.chain(english_numbers.items(), letters.items(), german_numbers.items())),
+    ]
+    assert result == expected