Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

fix modelbit response bug + making feature store value available in realtime feature component #44

Merged
merged 2 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "wyvern-ai"
version = "0.0.13"
version = "0.0.14"
description = ""
authors = ["Wyvern AI <info@wyvern.ai>"]
readme = "README.md"
Expand Down
16 changes: 8 additions & 8 deletions wyvern/components/features/feature_retrieval_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from ddtrace import tracer
from pydantic.generics import GenericModel

from wyvern import request_context
from wyvern.components.component import Component
from wyvern.components.features.feature_logger import (
FeatureEventLoggingComponent,
Expand All @@ -22,7 +23,7 @@
)
from wyvern.entities.candidate_entities import CandidateSetEntity
from wyvern.entities.feature_entities import FeatureData, FeatureMap
from wyvern.entities.feature_entity_helpers import feature_map_create, feature_map_join
from wyvern.entities.feature_entity_helpers import feature_map_create
from wyvern.entities.identifier_entities import WyvernEntity
from wyvern.wyvern_typing import REQUEST_ENTITY

Expand Down Expand Up @@ -143,6 +144,9 @@ async def execute(

all_entities = input.request.get_all_entities(cached=True)
all_identifiers = input.request.get_all_identifiers(cached=True)

current_request = request_context.ensure_current_request()

# TODO (suchintan): Pass in the feature retrieval features here so they can leverage them
feature_retrieval_request = FeatureStoreRetrievalRequest(
identifiers=all_identifiers,
Expand All @@ -156,6 +160,7 @@ async def execute(
**kwargs,
)
)
current_request.feature_map = feature_retrieval_response

"""
TODO (suchintan):
Expand Down Expand Up @@ -307,6 +312,7 @@ async def execute(
*real_time_candidate_features,
*real_time_candidate_combination_features,
)
current_request.extend_feature_map(real_time_feature_responses)

with tracer.trace("FeatureRetrievalPipeline.create_feature_response"):
await self.feature_logger_component.execute(
Expand All @@ -316,10 +322,4 @@ async def execute(
),
)

# TODO (suchintan): Improve performance of this
feature_responses = feature_map_join(
feature_retrieval_response,
real_time_feature_responses,
)

return feature_responses
return current_request.feature_map
8 changes: 4 additions & 4 deletions wyvern/components/models/modelbit_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,12 @@ async def inference(self, input: MODEL_INPUT, **kwargs) -> MODEL_OUTPUT:
output_data: Dict[Identifier, Optional[Union[float, str, List[float]]]] = {}

for batch_idx, resp in enumerate(responses):
if resp.status_code != 200:
if resp.status != 200:
logger.warning(f"Modelbit inference failed: {resp.text}")
continue
resp_list: List[
List[Union[float, str, List[float], None]]
] = resp.json().get(
resp_list: List[List[Union[float, str, List[float], None]]] = (
await resp.json()
).get(
"data",
[],
)
Expand Down
5 changes: 1 addition & 4 deletions wyvern/components/pipeline_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from functools import cached_property
from typing import Optional, Set, Type

from wyvern import request_context
from wyvern.components.api_route_component import APIRouteComponent
from wyvern.components.component import Component
from wyvern.components.features.feature_retrieval_pipeline import (
Expand Down Expand Up @@ -67,11 +66,9 @@ async def retrieve_features(self, request: REQUEST_ENTITY) -> None:
requested_feature_names=self.feature_names,
feature_overrides=self.realtime_features_overrides,
)
feature_map = await self.feature_retrieval_pipeline.execute(
await self.feature_retrieval_pipeline.execute(
feature_request,
)
current_request = request_context.ensure_current_request()
current_request.feature_map = feature_map

async def warm_up(self, input: REQUEST_ENTITY) -> None:
await super().warm_up(input)
Expand Down
3 changes: 3 additions & 0 deletions wyvern/wyvern_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,6 @@ def parse_fastapi_request(
feature_map=FeatureMap(feature_map={}),
request_id=request_id,
)

def extend_feature_map(self, feature_map: FeatureMap) -> None:
self.feature_map.feature_map.update(feature_map.feature_map)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any duplication check? If not, let's create a ticket for adding one, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of duplicate use case you're thinking of? is it overriding an existing key by another existing key?

The the self.feature_map.feature_map is already a python dictionary and it should have no duplicates.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that they define one offline, one real time feautre view and they both have the same name and same feature name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is going to screw up the mapping for sure. if that happens today, the realtime feature is going to override the offline feature from the feature store. 🤔