Only Build Features for Cohort [Resolves #513] #567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By default, only build/save features for the given cohort. This is to optimize speed for new users (and visit-based problems) to only what is necessary. For other users, the `save_all_features` flag is introduced. If this is set to True, the old behavior that builds features independently of the cohort is utilized. To make the use of this flag in combination with the replace Flag safe, the feature table "needs features?" check now sees if there are any cohort rows not present in the imputed table and rebuilds if anything is missing.
thcrock
force-pushed
the
cohort_features
branch
from
a852981
to
891dbe5
Compare
Codecov Report
@@ Coverage Diff @@
## master #567 +/- ##
==========================================
- Coverage 83.13% 83.07% -0.06%
==========================================
Files 83 83
Lines 4755 4780 +25
==========================================
+ Hits 3953 3971 +18
- Misses 802 809 +7
Continue to review full report at Codecov.
|
saleiro
reviewed
Jan 24, 2019
| JOIN {cohort_table} ON ( | ||
| {cohort_table.entity_id} = {from_obj.entity_id} | ||
| AND {cohort_table.date} = {as_of_time} | ||
| ) | ||
| WHERE {knowledge_date_column} >= {as_of_time} - interval {interval} | ||
| GROUP BY {group} | ||
| ``` |
There was a problem hiding this comment.
Yes! I tested with explain analyze and everything looks good!
saleiro
reviewed
Jan 24, 2019
| @@ -110,11 +110,16 @@ a column or SQL expression representing a numeric quantity present in the `from_ | |||
| of aggregate functions we want to use. The aggregate function is applied to the quantity. | |||
| * Each `group` is a column applied to the GROUP BY clause. Generally this is 'entity_id', but higher-level groupings | |||
| (for instance, 'zip_code') can be used as long as they can be rolled up to 'entity_id'. | |||
| * By default the query is joined with the cohort table (see 'state table' above) to remove unnecessary rows. If `save_all_features` is passed to the Experiment this is not done. | |||
|
|
|||
There was a problem hiding this comment.
Personally, save_all_features makes me think of columns and not rows... Maybe something like 'features_ignore_cohort'. or 'features_besides_cohort', or 'features_for_all' :) ?
There was a problem hiding this comment.
I agree. My preference is features_ignore_cohort, I'll change that
saleiro
approved these changes
Jan 24, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default, only build/save features for the given cohort. This is to
optimize speed for new users (and visit-based problems) to only what is
necessary.
For other users, the
save_all_featuresflag is introduced. If this isset to True, the old behavior that builds features independently of the
cohort is utilized.
To make the use of this flag in combination with the replace Flag safe,
the feature table "needs features?" check now sees if there are any
cohort rows not present in the imputed table and rebuilds if anything is
missing.