Repository navigation
Specify unique-row-id column in get_historical_features #1736
Description
Activity
Let me just rephrase this issue so that its a bit clearer. Do you agree with the following example?
You have the following
transactiontable+----------------+-----------+-------------+ | transaction_id | driver_id | customer_id | +----------------+-----------+-------------+ | 32398239 | 1 | 10001 | | 47584738 | 1 | 10001 | | 12378345 | 1 | 10003 | +----------------+-----------+-------------+and you have the following
drivertable+-----------+-----+-----+ | driver_id | f1 | f2 | +-----------+-----+-----+ | 1 | 1.0 | 1.2 | | 2 | 1.3 | 5.2 | | 3 | 2.4 | 1.2 | +-----------+-----+-----+and you have the following
customertable+-------------+-----+-----+ | customer_id | f3 | f4 | +-------------+-----+-----+ | 10001 | 5.0 | 4.5 | | 10002 | 6.3 | 2.4 | | 10003 | 8.4 | 1.6 | +-------------+-----+-----+then you somehow want to be able to produce a training dataset as follows
+----------------+-----------+-------------+-----+-----+-----+-----+ | transaction_id | driver_id | customer_id | f1 | f2 | f3 | f4 | +----------------+-----------+-------------+-----+-----+-----+-----+ | 32398239 | 1 | 10001 | 1.0 | 1.2 | 5.0 | 4.5 | | 47584738 | 1 | 10001 | 1.0 | 1.2 | 5.0 | 4.5 | | 12378345 | 1 | 10003 | 1.0 | 1.2 | 8.4 | 1.6 | +----------------+-----------+-------------+-----+-----+-----+-----+but for some reason you are either getting (A)
+----------------+-----------+-------------+-----+-----+-----+-----+ | transaction_id | driver_id | customer_id | f1 | f2 | f3 | f4 | +----------------+-----------+-------------+-----+-----+-----+-----+ | 47584738 | 1 | 10001 | 1.0 | 1.2 | 5.0 | 4.5 | | 12378345 | 1 | 10003 | 1.0 | 1.2 | 8.4 | 1.6 | +----------------+-----------+-------------+-----+-----+-----+-----+or (B)
+-----------+-------------+-----+-----+-----+-----+ | driver_id | customer_id | f1 | f2 | f3 | f4 | +-----------+-------------+-----+-----+-----+-----+ | 1 | 10001 | 1.0 | 1.2 | 5.0 | 4.5 | | 1 | 10001 | 1.0 | 1.2 | 5.0 | 4.5 | | 1 | 10003 | 1.0 | 1.2 | 8.4 | 1.6 | +-----------+-------------+-----+-----+-----+-----+If I understand you correctly, you are calling (B) the "duplicate rows". (Can you please clarify if the features are duplicated as well, or only the entity row?)
The behavior that I think Feast should have is to leave your entire entity dataframe intact. So you would provide
+----------------+-----------+-------------+ | transaction_id | driver_id | customer_id | +----------------+-----------+-------------+ | 32398239 | 1 | 10001 | | 47584738 | 1 | 10001 | | 12378345 | 1 | 10003 | +----------------+-----------+-------------+We would always return this left hand side of the join as-is. We will join on a subset of the entity columns (based on what each feature view needs for its join). Does that solve your problem? is Feast acting differently today?
Thanks Willem for this clarification 🙏
Based on my current understanding, it sounds counter intuitive to change the behaviour of
__entity_row_unique_id. Indeed, the duplicated data is part of theentity_dfthat you provide.If you want to perform an historical retrieval of just the drivers (or customers), I would advise you to GROUP BY the data in your
entity_dffirst.For example at Shopify, we generate a SQL query for our users that will GROUP BY the data in the entity dataframe first
# Snippet of a function that generate the `entity_df` SQL query for our users SELECT {', '.join(unique_join_keys)}, TIMESTAMP '{str_timestamp}' AS {timestamp_column} FROM {source_table} {where_clause} GROUP BY {', '.join(unique_join_keys)} {limit_clause}
Which would give you something like the following in your use case
# Snippet of a function that generate the `entity_df` SQL query for our users SELECT transaction_id, driver_id, timestamp_column FROM {source_table} {where_clause} GROUP BY transaction_id, driver_id, {limit_clause}
Let me know if that makes sense and/or I am missing some context here
Reacted by Willem Pienaarthanks for responses. To build on top of @woop's clarification:
If this was the entity source data:
+----------------+-----------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+ | transaction_id (not a feast entity) | non_feast_data (not a feast entity) | driver_id (an entity for feast join) | customer_id (an entity for feast join) +----------------+-----------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+ | 32398239 | "a1" | 1 | 10001 | 47584738 | "a2" | 1 | 10001 | 12378345 | "a3" | 1 | 10003 +----------------+-----------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+Note here that the first two columns aren't feast entities but data that we want to preserve and data we already know is unique per row.
At this point, every row is unique but the feast-related columns [driver_id, customer_id] (e.g (1, 10001) in the table) per row are not. The current behavior with this table is duplication that outputs a table often greater than the number of rows started with. E.g. 10 MM becomes 23 MM.
If we first force the [driver_id, customer_id] to be unique and do a pre-historical-retrieval groupby, feast's historical retrieval would behave as expected, but through this deduplication preprocessing either row # 1 or 2 would be dropped, which is not desirable since these two rows contain unique information though not to Feast.
I think timestamp is also part of the uniqueness consideration, but often these timestamps (e.g. for row # 1 and 2) would be the same especially in cases in which we do timestamp truncation for the purpose of joining by hour of day.
If I understand correctly @MattDelac, is your sql query for the purpose of eliminating duplicates but would still run into the issue described above in this msg?
The current behavior with this table is duplication that outputs a table often greater than the number of rows started with. E.g. 10 MM becomes 23 MM.
That sounds like a bug then. Your entity dataframe should just be enriched with a join of feature values.
Do you have a way for us to reproduce this problem? Perhaps a feature repo using parquet files? Or is it specific to BigQuery?
PS: You can use https://ozh.github.io/ascii-tables/ for creating tables.
I see. So I feel that the problem is the fact that the data used by Feast is not unique. We can look at how we generate the UUID of each row as the problem might come from there. The problem being that
ROW_NUMBER OVER()is not a good solution at scale norGENERATE_UUID()would be (because it's not deterministic).So my first instinct tells me that we should generate an UUID when saving the entity_dataframe into BigQuery.
But first, let's add more unit tests so that we can make sure it's covered
I don't have a way to reproduce this bug, but I believe you can see the potential for duplication by inspecting the final part of the SQL query we use for
get_historical_features:SELECT * EXCEPT(entity_timestamp, {% for featureview in featureviews %} {{featureview.name}}__entity_row_unique_id{% if loop.last %}{% else %},{% endif %}{% endfor %}) FROM entity_dataframe {% for featureview in featureviews %} LEFT JOIN ( SELECT {{featureview.name}}__entity_row_unique_id {% for feature in featureview.features %} ,{% if full_feature_names %}{{ featureview.name }}__{{feature}}{% else %}{{ feature }}{% endif %} {% endfor %} FROM {{ featureview.name }}__cleaned ) USING ({{featureview.name}}__entity_row_unique_id) {% endfor %}We take the spine (
entity_dataframe) and enrich it by doing aLEFT JOINwith feature data from the offline store. For each feature view, theLEFT JOINchecks for equality on{{featureview.name}}__entity_row_unique_id; if this is not unique, then theLEFT JOINwill produce extra rows. I'm assuming that in @mavysavydav 's example, since (A, B, C, event_timestamp) is not unique, this exact issue is occurring.Reacted by Matt Delacour and Willem Pienaarthanks, will think over that and get back to you guys. And thanks for link for producing ascii tables willem hahah
@mavysavydav Which version of Feast do you use ? I am wondering if the following change would fix your issue ? #1712
yes @MattDelac ! I had thought that your change would just optimize the query run (which it does), but now that I try it against this duplication issue, it's resolved that too. Upon a closer look, it seems like it's b/c of the groupby you added. In the older version of the sql, we groupby the unique entity row id for the intermediary generated featureview tables at every stage except we don't do it for the creation of {{feature_view.name}}__cleaned, which are the feature tables that are joined to the entity source data. So if the entity source data has duplicates, there's a good chance that the intermediary feature tables have duplicates, and these duplicates multiply out causing an explosion. Thanks for your PR!
Reacted by Matt Delacour
Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
If I have columns X, A, B, C, event_timestamp in my entity source data and A, B, C are the entity columns to join feature data to but the combination of [A, B, C, event_timestamp] may not be unique, the join will have issues that produces duplicate rows. One solution is to preprocess the data so that only the unique rows of the combination are filtered out for the join, but we may want all the rows to be preserved since X may already be unique and each row represents a real unique training example. It could also be that there are columns Y, Z, etc that aren't part of the feast join but contain unique info on a row basis so it doesn't make sense to filter those out. In this example, X might be impression_id for instance and we're not joining data directly based on impression_id but based on the A, B, C columns which might be tweet_id, user_id, etc.
Describe the solution you'd like
A clear and concise description of what you want to happen.
Be able to optionally specify a unique-row-id column in get_historical_features so in the example above, X would be chosen as the unique-row-id column. I've tested swapping this part of the feast join query
CONCAT( {% for entity in featureview.entities %} CAST({{entity}} AS STRING), {% endfor %} CAST({{entity_df_event_timestamp_col}} AS STRING) ) AS {{featureview.name}}__entity_row_unique_id,with just
X as entity_row_unique_idand it fixes the issue, plus there seems to be performance gains possibly from just eliminating the work of creating the concatenated strings for each row. I think the change should be relatively easy to make though this involves an API change which always requires some consideration. get_historical_features might become something like
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
N/A
Additional context
Add any other context or screenshots about the feature request here.