From cb203c8aac47b95475f8020b787f32add5122e55 Mon Sep 17 00:00:00 2001 From: MalvinaNikandrou Date: Tue, 5 Dec 2023 02:20:52 +0000 Subject: [PATCH] Remove confirmation --- .env.simbot | 4 - .../api/clients/__init__.py | 3 - .../confirmation_response_classifier.py | 27 --- .../api/controllers/simbot/clients.py | 12 +- .../api/controllers/simbot/controller.py | 21 +- .../api/controllers/simbot/pipelines.py | 8 +- .../common/settings/simbot.py | 17 -- .../datamodels/simbot/__init__.py | 1 - .../datamodels/simbot/enums/__init__.py | 1 - .../datamodels/simbot/enums/intents.py | 43 +---- .../datamodels/simbot/session.py | 14 +- .../simbot/agent_intent_selection/__init__.py | 4 - .../confirmation_handler.py | 182 ------------------ .../instruction_handler.py | 23 --- .../functions/simbot/grab_from_history.py | 27 +-- .../parsers/simbot/previous_action.py | 5 +- .../simbot/agent_intent_selection.py | 46 ++--- .../simbot/user_intent_extraction.py | 40 +--- 18 files changed, 37 insertions(+), 441 deletions(-) delete mode 100644 src/emma_experience_hub/api/clients/confirmation_response_classifier.py delete mode 100644 src/emma_experience_hub/functions/simbot/agent_intent_selection/confirmation_handler.py diff --git a/.env.simbot b/.env.simbot index 0f5aa93d..7cac7b4f 100644 --- a/.env.simbot +++ b/.env.simbot @@ -1,7 +1,3 @@ SIMBOT_AUXILIARY_METADATA_DIR='./storage/simbot/fixtures/game_metadata' SIMBOT_EXTRACTED_FEATURES_DIR='./storage/fixtures/simbot/features' - -SIMBOT_AUXILIARY_METADATA_S3_BUCKET="emma-simbot-live-challenge" - -SIMBOT_FEATURE_EXTRACTOR_CHECKPOINT_URI='s3://emma-simbot/model/checkpoints/vinvl_vg_x152c4_simbot.pth' diff --git a/src/emma_experience_hub/api/clients/__init__.py b/src/emma_experience_hub/api/clients/__init__.py index 82dd5c46..312e804e 100644 --- a/src/emma_experience_hub/api/clients/__init__.py +++ b/src/emma_experience_hub/api/clients/__init__.py @@ -1,6 +1,3 @@ from emma_experience_hub.api.clients.client import Client -from emma_experience_hub.api.clients.confirmation_response_classifier import ( - ConfirmationResponseClassifierClient, -) from emma_experience_hub.api.clients.emma_policy import EmmaPolicyClient from emma_experience_hub.api.clients.feature_extractor import FeatureExtractorClient diff --git a/src/emma_experience_hub/api/clients/confirmation_response_classifier.py b/src/emma_experience_hub/api/clients/confirmation_response_classifier.py deleted file mode 100644 index 77299c28..00000000 --- a/src/emma_experience_hub/api/clients/confirmation_response_classifier.py +++ /dev/null @@ -1,27 +0,0 @@ -from typing import Optional - -import httpx -from loguru import logger - -from emma_experience_hub.api.clients.client import Client - - -class ConfirmationResponseClassifierClient(Client): - """Client for the confirmation response classifier.""" - - def healthcheck(self) -> bool: - """Verify the server is healthy.""" - return self._run_healthcheck(f"{self._endpoint}/healthcheck") - - def is_request_approved(self, text: str) -> Optional[bool]: - """Return boolean to the incoming request, or None if it is not a confirmation request.""" - with httpx.Client(timeout=self._timeout) as client: - response = client.post(f"{self._endpoint}/is-confirmation", params={"text": text}) - - try: - response.raise_for_status() - except httpx.HTTPError as err: - logger.exception("Unable to detect whether utterance is a confirmation response") - raise err from None - - return response.json() diff --git a/src/emma_experience_hub/api/controllers/simbot/clients.py b/src/emma_experience_hub/api/controllers/simbot/clients.py index a0028d61..655b00ac 100644 --- a/src/emma_experience_hub/api/controllers/simbot/clients.py +++ b/src/emma_experience_hub/api/controllers/simbot/clients.py @@ -7,11 +7,7 @@ from loguru import logger from pydantic import BaseModel -from emma_experience_hub.api.clients import ( - Client, - ConfirmationResponseClassifierClient, - FeatureExtractorClient, -) +from emma_experience_hub.api.clients import Client, FeatureExtractorClient from emma_experience_hub.api.clients.simbot import ( SimbotActionPredictionClient, SimBotAuxiliaryMetadataClient, @@ -33,7 +29,6 @@ class SimBotControllerClients(BaseModel, arbitrary_types_allowed=True): nlu_intent: SimBotNLUIntentClient action_predictor: SimbotActionPredictionClient session_db: SimBotSessionDbClient - confirmation_response_classifier: ConfirmationResponseClassifierClient @classmethod def from_simbot_settings(cls, simbot_settings: SimBotSettings) -> "SimBotControllerClients": @@ -66,11 +61,6 @@ def from_simbot_settings(cls, simbot_settings: SimBotSettings) -> "SimBotControl endpoint=simbot_settings.action_predictor_url, timeout=simbot_settings.client_timeout, ), - confirmation_response_classifier=ConfirmationResponseClassifierClient( - endpoint=simbot_settings.confirmation_classifier_url, - timeout=simbot_settings.client_timeout, - disable=not simbot_settings.feature_flags.enable_confirmation_questions, - ), ) def healthcheck(self, attempts: int = 1, interval: int = 0) -> bool: diff --git a/src/emma_experience_hub/api/controllers/simbot/controller.py b/src/emma_experience_hub/api/controllers/simbot/controller.py index 281fafa0..fcfa4c61 100644 --- a/src/emma_experience_hub/api/controllers/simbot/controller.py +++ b/src/emma_experience_hub/api/controllers/simbot/controller.py @@ -212,24 +212,9 @@ def _clear_queue_if_needed(self, session: SimBotSession) -> SimBotSession: return session def _clear_queue_after_user_intent(self, session: SimBotSession) -> SimBotSession: - """Clear the queue if the user has provided us with a new instruction. - - After the confirmation classifier, check if the user replied to a confirmation question - with a confirmation response or a new instruction. - """ - new_instruction_after_confirmation = [ - # We previously asked for confirmation - session.previous_turn is not None - and session.previous_turn.intent.verbal_interaction is not None - and session.previous_turn.intent.verbal_interaction.type.triggers_confirmation_question, - # But the user intent is not a confirmation response - session.current_turn.intent.user is not None - and not session.current_turn.intent.user.is_confirmation_response, - ] - if all(new_instruction_after_confirmation): - logger.debug( - "[REQUEST]: Received instruction utterance after confirmation; clearing the utterance queue" - ) + """Clear the queue if the user has provided us with a new instruction.""" + if session.current_turn.intent.user is not None: + logger.debug("[REQUEST]: Received instruction utterance; clearing the utterance queue") session.current_state.utterance_queue.reset() session.current_state.find_queue.reset() diff --git a/src/emma_experience_hub/api/controllers/simbot/pipelines.py b/src/emma_experience_hub/api/controllers/simbot/pipelines.py index f309d384..45c6f1e3 100644 --- a/src/emma_experience_hub/api/controllers/simbot/pipelines.py +++ b/src/emma_experience_hub/api/controllers/simbot/pipelines.py @@ -50,11 +50,7 @@ def from_clients( request_processing=SimBotRequestProcessingPipeline( session_db_client=clients.session_db, ), - user_intent_extractor=SimBotUserIntentExtractionPipeline( - confirmation_response_classifier=clients.confirmation_response_classifier, - _enable_confirmation_questions=simbot_settings.feature_flags.enable_confirmation_questions, - _enable_clarification_questions=simbot_settings.feature_flags.enable_clarification_questions, - ), + user_intent_extractor=SimBotUserIntentExtractionPipeline(), environment_intent_extractor=SimBotEnvironmentIntentExtractionPipeline(), agent_intent_selector=SimBotAgentIntentSelectionPipeline( features_client=clients.features, @@ -65,11 +61,9 @@ def from_clients( environment_error_pipeline=SimBotEnvironmentErrorCatchingPipeline(), action_predictor_client=clients.action_predictor, _enable_clarification_questions=simbot_settings.feature_flags.enable_clarification_questions, - _enable_confirmation_questions=simbot_settings.feature_flags.enable_confirmation_questions, _enable_search_actions=simbot_settings.feature_flags.enable_search_actions, _enable_search_after_no_match=simbot_settings.feature_flags.enable_search_after_no_match, _enable_search_after_missing_inventory=simbot_settings.feature_flags.enable_search_after_missing_inventory, - _enable_high_level_planner=simbot_settings.feature_flags.enable_rasa_high_level_planner, ), agent_action_generator=SimBotAgentActionGenerationPipeline( features_client=clients.features, diff --git a/src/emma_experience_hub/common/settings/simbot.py b/src/emma_experience_hub/common/settings/simbot.py index fe25208a..17a47ab5 100644 --- a/src/emma_experience_hub/common/settings/simbot.py +++ b/src/emma_experience_hub/common/settings/simbot.py @@ -19,18 +19,12 @@ class SimBotFeatureFlags(BaseModel): enable_always_highlight_before_object_action: bool = False enable_clarification_questions: bool = True - enable_coreference_resolution: bool = True - enable_confirmation_questions: bool = True enable_grab_from_history: bool = True - enable_incomplete_utterances_intent: bool = True - enable_rasa_high_level_planner: bool = False enable_scanning_during_search: bool = True enable_search_actions: bool = True enable_search_after_missing_inventory: bool = True enable_search_after_no_match: bool = True - prevent_default_response_as_lightweight: bool = True - search_planner_type: SearchPlannerType = SearchPlannerType.greedy_max_vertex_cover gfh_location_type: GFHLocationType = GFHLocationType.location @@ -43,12 +37,8 @@ def set_offline_evaluation_flags( """Make sure that flags are set correctly for the offline evaluation.""" if values.get("enable_offline_evaluation", False): values["enable_clarification_questions"] = False - values["enable_coreference_resolution"] = False - values["enable_confirmation_questions"] = False - values["enable_incomplete_utterances_intent"] = False values["enable_scanning_during_search"] = False values["enable_search_after_missing_inventory"] = False - values["prevent_default_response_as_lightweight"] = False values["gfh_location_type"] = GFHLocationType.viewpoint return values @@ -66,8 +56,6 @@ class SimBotSettings(BaseSettings): client_timeout: Optional[int] = 5 - simbot_cache_s3_bucket: str = "emma-simbot-live-challenge" - auxiliary_metadata_dir: DirectoryPath auxiliary_metadata_cache_dir: DirectoryPath @@ -75,7 +63,6 @@ class SimBotSettings(BaseSettings): session_db_memory_table_name: str = "SIMBOT_MEMORY_TABLE" session_local_db_file: str = "storage/local_sessions.db" - session_db_region: str = "us-east-1" feature_extractor_url: AnyHttpUrl = AnyHttpUrl(url=f"{scheme}://0.0.0.0:5500", scheme=scheme) @@ -84,10 +71,6 @@ class SimBotSettings(BaseSettings): action_predictor_url: AnyHttpUrl = AnyHttpUrl(url=f"{scheme}://0.0.0.0:5502", scheme=scheme) - confirmation_classifier_url: AnyHttpUrl = AnyHttpUrl( - url=f"{scheme}://0.0.0.0:5507", scheme=scheme - ) - placeholder_vision_url: AnyHttpUrl = AnyHttpUrl(url=f"{scheme}://0.0.0.0:5506", scheme=scheme) class Config: diff --git a/src/emma_experience_hub/datamodels/simbot/__init__.py b/src/emma_experience_hub/datamodels/simbot/__init__.py index 11203b80..78d2dc05 100644 --- a/src/emma_experience_hub/datamodels/simbot/__init__.py +++ b/src/emma_experience_hub/datamodels/simbot/__init__.py @@ -6,7 +6,6 @@ from emma_experience_hub.datamodels.simbot.enums import ( SimBotActionStatusType, SimBotActionType, - SimBotAnyUserIntentType, SimBotDummyRawActions, SimBotEnvironmentIntentType, SimBotIntentType, diff --git a/src/emma_experience_hub/datamodels/simbot/enums/__init__.py b/src/emma_experience_hub/datamodels/simbot/enums/__init__.py index e8ea284d..47bc3248 100644 --- a/src/emma_experience_hub/datamodels/simbot/enums/__init__.py +++ b/src/emma_experience_hub/datamodels/simbot/enums/__init__.py @@ -4,7 +4,6 @@ SimBotDummyRawActions, ) from emma_experience_hub.datamodels.simbot.enums.intents import ( - SimBotAnyUserIntentType, SimBotEnvironmentIntentType, SimBotIntentType, SimBotNLUIntentType, diff --git a/src/emma_experience_hub/datamodels/simbot/enums/intents.py b/src/emma_experience_hub/datamodels/simbot/enums/intents.py index 08af5e7d..a1f7f727 100644 --- a/src/emma_experience_hub/datamodels/simbot/enums/intents.py +++ b/src/emma_experience_hub/datamodels/simbot/enums/intents.py @@ -20,18 +20,7 @@ class SimBotIntentType(Enum): act_too_many_matches = "" act_missing_inventory = "" - # Confirmation question triggers - confirm_generic = "" - confirm_before_act = "" - confirm_before_goto_object = "" - confirm_before_goto_viewpoint = "" - confirm_before_search = "" - confirm_before_plan = "" - - # Question responses from the user clarify_answer = "" - confirm_yes = "" - confirm_no = "" # Feedback for previous turn success generic_success = "" @@ -83,33 +72,13 @@ def is_actionable(self) -> bool: @property def triggers_question_to_user(self) -> bool: """Return True if the intent triggers a question to the user.""" - return self.triggers_confirmation_question or self.triggers_disambiguation_question + return self.triggers_disambiguation_question @property def triggers_disambiguation_question(self) -> bool: """Did the agent ask for disambiguation from the user?""" return self == SimBotIntentType.act_too_many_matches - @property - def triggers_confirmation_question(self) -> bool: - """Did the agent ask for confirmation from the user?""" - return self in { - SimBotIntentType.confirm_generic, - SimBotIntentType.confirm_before_act, - SimBotIntentType.confirm_before_goto_object, - SimBotIntentType.confirm_before_goto_viewpoint, - SimBotIntentType.confirm_before_search, - SimBotIntentType.confirm_before_plan, - } - - @property - def is_confirmation_response(self) -> bool: - """Return True if the intent is a response to a confirmation question.""" - return self in { - SimBotIntentType.confirm_yes, - SimBotIntentType.confirm_no, - } - @property def verbal_interaction_intent_triggers_search(self) -> bool: """Return True if the intent is a verbal interaction intent that triggers search.""" @@ -156,14 +125,10 @@ def is_verbal_interaction_intent_type( # noqa: WPS602 SimBotUserIntentType = Literal[ SimBotIntentType.clarify_answer, - SimBotIntentType.confirm_yes, - SimBotIntentType.confirm_no, SimBotIntentType.act, ] -SimBotAnyUserIntentType = Literal[SimBotUserIntentType,] - SimBotEnvironmentIntentType = Literal[ SimBotIntentType.unsupported_action, SimBotIntentType.unsupported_navigation, @@ -194,12 +159,6 @@ def is_verbal_interaction_intent_type( # noqa: WPS602 ] SimBotVerbalInteractionIntentType = Literal[ - SimBotIntentType.confirm_generic, - SimBotIntentType.confirm_before_act, - SimBotIntentType.confirm_before_goto_object, - SimBotIntentType.confirm_before_goto_viewpoint, - SimBotIntentType.confirm_before_search, - SimBotIntentType.confirm_before_plan, SimBotIntentType.act_no_match, SimBotIntentType.act_too_many_matches, SimBotIntentType.act_missing_inventory, diff --git a/src/emma_experience_hub/datamodels/simbot/session.py b/src/emma_experience_hub/datamodels/simbot/session.py index 027db3cf..0ccde1a6 100644 --- a/src/emma_experience_hub/datamodels/simbot/session.py +++ b/src/emma_experience_hub/datamodels/simbot/session.py @@ -21,10 +21,10 @@ from emma_experience_hub.datamodels.simbot.agent_memory import SimBotInventory, SimBotObjectMemory from emma_experience_hub.datamodels.simbot.enums import ( SimBotActionType, - SimBotAnyUserIntentType, SimBotEnvironmentIntentType, SimBotIntentType, SimBotPhysicalInteractionIntentType, + SimBotUserIntentType, SimBotVerbalInteractionIntentType, ) from emma_experience_hub.datamodels.simbot.intents import SimBotIntent @@ -58,7 +58,7 @@ def processing_time(self) -> Optional[float]: class SimBotSessionTurnIntent(BaseModel): """Intents from each actor within the environment for the turn..""" - user: Optional[SimBotAnyUserIntentType] = None + user: Optional[SimBotUserIntentType] = None environment: Optional[SimBotIntent[SimBotEnvironmentIntentType]] = None physical_interaction: Optional[SimBotIntent[SimBotPhysicalInteractionIntentType]] = None verbal_interaction: Optional[SimBotIntent[SimBotVerbalInteractionIntentType]] = None @@ -88,7 +88,7 @@ def should_generate_interaction_action(self) -> bool: @property def agent_should_ask_question_to_user(self) -> bool: - """Return True if the agent should ask for confirmation instead of just acting.""" + """Return True if the agent should ask instead of just acting.""" return ( self.verbal_interaction is not None and self.verbal_interaction.type.triggers_question_to_user @@ -96,11 +96,10 @@ def agent_should_ask_question_to_user(self) -> bool: @property def agent_should_ask_question_without_acting(self) -> bool: - """Return True if the agent should ask for confirmation instead of just acting.""" + """Return True if the agent should ask instead of just acting.""" return ( self.verbal_interaction is not None and self.verbal_interaction.type.triggers_question_to_user - and self.verbal_interaction.type != SimBotIntentType.confirm_before_plan ) @property @@ -375,11 +374,6 @@ def convert_to_simbot_response(self) -> SimBotResponse: """Convert the session turn to a SimBotResponse, to be returned to the API.""" actions: list[SimBotAction] = self.actions.to_list() - # If the agent wants to ask for confirmation before trying to act, then only return the - # dialog action, which SHOULD have the confirmation question for the user - if self.intent.agent_should_ask_question_without_acting: - actions = [self.actions.dialog] if self.actions.dialog is not None else [] - if not actions: raise AssertionError( "There is no action to be returned. Have you run the response generator on this?" diff --git a/src/emma_experience_hub/functions/simbot/agent_intent_selection/__init__.py b/src/emma_experience_hub/functions/simbot/agent_intent_selection/__init__.py index 9a0db7c1..eb2ceeb9 100644 --- a/src/emma_experience_hub/functions/simbot/agent_intent_selection/__init__.py +++ b/src/emma_experience_hub/functions/simbot/agent_intent_selection/__init__.py @@ -1,10 +1,6 @@ from emma_experience_hub.functions.simbot.agent_intent_selection.clarification_handler import ( SimBotClarificationHandler, ) -from emma_experience_hub.functions.simbot.agent_intent_selection.confirmation_handler import ( - SimBotConfirmationHandler, - set_find_object_in_progress_intent, -) from emma_experience_hub.functions.simbot.agent_intent_selection.instruction_handler import ( SimBotActHandler, ) diff --git a/src/emma_experience_hub/functions/simbot/agent_intent_selection/confirmation_handler.py b/src/emma_experience_hub/functions/simbot/agent_intent_selection/confirmation_handler.py deleted file mode 100644 index e10ba081..00000000 --- a/src/emma_experience_hub/functions/simbot/agent_intent_selection/confirmation_handler.py +++ /dev/null @@ -1,182 +0,0 @@ -from typing import Optional - -from emma_common.datamodels import SpeakerRole -from emma_experience_hub.datamodels.simbot import ( - SimBotAgentIntents, - SimBotIntent, - SimBotIntentType, - SimBotSession, - SimBotSessionTurn, - SimBotUserIntentType, - SimBotUserSpeech, -) -from emma_experience_hub.datamodels.simbot.queue import SimBotQueueUtterance - - -def set_find_object_in_progress_intent(session: SimBotSession) -> SimBotAgentIntents: - """Set the intent when find is in progress.""" - if not session.previous_turn or session.previous_turn.intent.physical_interaction is None: - return SimBotAgentIntents(SimBotIntent(type=SimBotIntentType.search)) - - entity = session.previous_turn.intent.physical_interaction.entity - previous_intent = session.previous_turn.intent - # Retain the information that the search was triggered by an act_no_match or act_missing_inventory - is_searching_inferred_object = ( - previous_intent.is_searching_inferred_object - and not session.previous_turn.is_going_to_found_object_from_search - ) - if is_searching_inferred_object and previous_intent.verbal_interaction is not None: - return SimBotAgentIntents( - SimBotIntent(type=SimBotIntentType.search, entity=entity), - SimBotIntent( - type=previous_intent.verbal_interaction.type, - entity=entity, - ), - ) - return SimBotAgentIntents(SimBotIntent(type=SimBotIntentType.search, entity=entity)) - - -class SimBotConfirmationHandler: - """Determine the agent intents after a confirmation.""" - - def __init__( - self, - _enable_search_after_no_match: bool = True, - ) -> None: - self._enable_search_after_no_match = _enable_search_after_no_match - - def run(self, session: SimBotSession) -> Optional[SimBotAgentIntents]: - """Handle a confirmation intent.""" - user_intent = session.current_turn.intent.user - if user_intent is None or not SimBotIntentType.is_user_intent_type(user_intent): - raise AssertionError("User intent should not be None!") - # If the agent asked for confirmation to search for an object required to act - if self._agent_asked_for_confirm_before_searching(session, user_intent): - return self._handle_confirm_before_search_intent( - session=session, user_intent=user_intent - ) - - # If we are within a find routine AND received a confirmation response from the user - if session.is_find_object_in_progress and user_intent.is_confirmation_response: - # Then let the search routine decide how to handle it. - return set_find_object_in_progress_intent(session) - - # If the agent explicitly asked a confirmation question before executing a plan in the previous turn - if self._agent_asked_for_confirm_before_plan(session.previous_valid_turn): - return self._handle_confirm_before_plan_intent(session, user_intent) - - # If the agent explicitly asked a confirmation question in the previous turn - if self._agent_asked_for_confirm_before_acting(session.previous_valid_turn): - return self._handle_confirm_before_previous_act_intent(session, user_intent) - return SimBotAgentIntents() - - def _handle_confirm_before_previous_act_intent( - self, session: SimBotSession, user_intent: SimBotUserIntentType - ) -> SimBotAgentIntents: - """Handle a confirmation to execute the previous action.""" - # If the user approved - if user_intent == SimBotIntentType.confirm_yes: - return SimBotAgentIntents( - physical_interaction=SimBotIntent(type=SimBotIntentType.act_previous) - ) - - session.current_state.utterance_queue.reset() - return SimBotAgentIntents() - - def _handle_confirm_before_search_intent( - self, session: SimBotSession, user_intent: SimBotUserIntentType - ) -> SimBotAgentIntents: - """Handle a confirmation to search.""" - if user_intent != SimBotIntentType.confirm_yes: - session.current_state.utterance_queue.reset() - return SimBotAgentIntents() - - previous_turn = session.previous_turn - if previous_turn is None or previous_turn.intent.verbal_interaction is None: - raise AssertionError("User intent is confirmation without a previous verbal intent") - - # Do a search routine before executing the latest instruction. - if not session.current_turn.speech.utterance.startswith("go to the"): # type: ignore[union-attr] - session.current_state.utterance_queue.append_to_head( - SimBotQueueUtterance( - utterance=previous_turn.speech.utterance, # type: ignore[union-attr] - role=previous_turn.speech.role, # type: ignore[union-attr] - ), - ) - target_entity = previous_turn.intent.verbal_interaction.entity - session.current_turn.speech = SimBotUserSpeech.update_user_utterance( - utterance=f"find the {target_entity}", - role=SpeakerRole.agent, - original_utterance=session.current_turn.speech.original_utterance - if session.current_turn.speech - else None, - ) - return SimBotAgentIntents( - physical_interaction=SimBotIntent(type=SimBotIntentType.search, entity=target_entity), - verbal_interaction=SimBotIntent( - type=SimBotIntentType.act_no_match, entity=target_entity - ), - ) - - def _handle_confirm_before_plan_intent( - self, session: SimBotSession, user_intent: SimBotUserIntentType - ) -> Optional[SimBotAgentIntents]: - """Handle a confirmation to execute the plan.""" - # If the user approved - if user_intent == SimBotIntentType.confirm_yes: - # Pop the first element in the instruction plan and add it to the utterance speech - queue_elem = session.current_state.utterance_queue.pop_from_head() - session.current_turn.speech = SimBotUserSpeech.update_user_utterance( - utterance=queue_elem.utterance, - from_utterance_queue=True, - role=queue_elem.role, - original_utterance=session.current_turn.speech.original_utterance - if session.current_turn.speech - else None, - ) - return None - - # If the user didn't approve - session.current_state.utterance_queue.reset() - return SimBotAgentIntents() - - def _agent_asked_for_confirm_before_acting( - self, previous_turn: Optional[SimBotSessionTurn] - ) -> bool: - """Did the agent explicitly ask for confirmation before performing an action?""" - return ( - previous_turn is not None - and previous_turn.intent.verbal_interaction is not None - and previous_turn.intent.verbal_interaction.type.triggers_confirmation_question - ) - - def _agent_asked_for_confirm_before_searching( - self, session: SimBotSession, user_intent: SimBotUserIntentType - ) -> bool: - """Did the agent explicitly ask for confirmation before searching for an unseen object?""" - if not self._enable_search_after_no_match: - return False - - # Verify the user provided a confirmation response (Y/N) - if not user_intent.is_confirmation_response or session.previous_turn is None: - return False - - # Verify the previous turn outputted an `confirm_before_search` - previous_verbal_intent = session.previous_turn.intent.verbal_interaction - is_previous_turn_confirm_before_search = ( - previous_verbal_intent is not None - and previous_verbal_intent.type == SimBotIntentType.confirm_before_search - ) - - return is_previous_turn_confirm_before_search - - def _agent_asked_for_confirm_before_plan( - self, previous_turn: Optional[SimBotSessionTurn] - ) -> bool: - """Did the agent explicitly ask for confirmation before performing an action?""" - return ( - previous_turn is not None - and previous_turn.intent.verbal_interaction is not None - and previous_turn.intent.verbal_interaction.type - == SimBotIntentType.confirm_before_plan - ) diff --git a/src/emma_experience_hub/functions/simbot/agent_intent_selection/instruction_handler.py b/src/emma_experience_hub/functions/simbot/agent_intent_selection/instruction_handler.py index 7d42aa9b..6fc90629 100644 --- a/src/emma_experience_hub/functions/simbot/agent_intent_selection/instruction_handler.py +++ b/src/emma_experience_hub/functions/simbot/agent_intent_selection/instruction_handler.py @@ -31,7 +31,6 @@ def __init__( nlu_intent_parser: NeuralParser[SimBotIntent[SimBotNLUIntentType]], action_predictor_client: SimbotActionPredictionClient, _enable_clarification_questions: bool = True, - _enable_confirmation_questions: bool = True, _enable_search_actions: bool = True, _enable_search_after_no_match: bool = True, _enable_search_after_missing_inventory: bool = True, @@ -44,7 +43,6 @@ def __init__( self._action_predictor_client = action_predictor_client self._enable_clarification_questions = _enable_clarification_questions - self._enable_confirmation_questions = _enable_confirmation_questions self._enable_search_actions = _enable_search_actions self._enable_search_after_no_match = _enable_search_after_no_match self._enable_missing_inventory = _enable_search_after_missing_inventory @@ -161,13 +159,6 @@ def _handle_act_no_match_intent( if target_entity is None: return intents - # Confirm the search if we have not seen the object before - if self._should_confirm_before_search(session, target_entity): - return SimBotAgentIntents( - verbal_interaction=SimBotIntent( - type=SimBotIntentType.confirm_before_search, entity=target_entity - ), - ) # Otherwise start the search if not session.current_turn.speech.utterance.startswith("go to the"): # type: ignore[union-attr] queue_elem = SimBotQueueUtterance( @@ -264,13 +255,9 @@ def _handle_search_holding_object( # If yes, get the object we will search for search_object = None physical_interaction_intent = intents.physical_interaction - verbal_interaction_intent = intents.verbal_interaction if physical_interaction_intent is not None: if physical_interaction_intent.type == SimBotIntentType.search: search_object = physical_interaction_intent.entity - elif verbal_interaction_intent is not None: - if verbal_interaction_intent.type == SimBotIntentType.confirm_before_search: - search_object = verbal_interaction_intent.entity # If we are not searching for an object, continue with the extracted intents if search_object is None: @@ -287,16 +274,6 @@ def _handle_search_holding_object( return SimBotAgentIntents() return intents - def _should_confirm_before_search(self, session: SimBotSession, target_entity: str) -> bool: - """Should the agent ask for confirmation before searching?""" - if not self._enable_confirmation_questions: - return False - # Don't ask if we've seen the entity in the current room or know its location from prior memory - has_seen_object = session.current_state.memory.object_in_memory( - target_entity, current_room=session.current_turn.environment.current_room - ) - return not has_seen_object - def _should_search_target_object( self, session: SimBotSession, intents: SimBotAgentIntents ) -> bool: diff --git a/src/emma_experience_hub/functions/simbot/grab_from_history.py b/src/emma_experience_hub/functions/simbot/grab_from_history.py index ac53481e..3bf6c317 100644 --- a/src/emma_experience_hub/functions/simbot/grab_from_history.py +++ b/src/emma_experience_hub/functions/simbot/grab_from_history.py @@ -2,8 +2,7 @@ from emma_common.datamodels import SpeakerRole from emma_experience_hub.constants.model import END_OF_TRAJECTORY_TOKEN, PREDICTED_ACTION_DELIMITER -from emma_experience_hub.constants.simbot import ROOM_SYNONYNMS -from emma_experience_hub.datamodels.simbot import SimBotIntent, SimBotIntentType, SimBotSession +from emma_experience_hub.datamodels.simbot import SimBotSession from emma_experience_hub.datamodels.simbot.actions import SimBotAction from emma_experience_hub.datamodels.simbot.enums import SimBotActionType from emma_experience_hub.datamodels.simbot.payloads import SimBotGotoRoom, SimBotGotoRoomPayload @@ -59,30 +58,6 @@ def _goto_room_before_search( ) return [self._create_goto_room_action(room)] - def _ask_for_confirmation_before_search( - self, session: SimBotSession, room: str, searchable_object: str - ) -> list[SimBotAction]: - """Ask confirmation before moving to the correct room based on prior memory.""" - if session.current_turn.speech is not None: - utterance = session.current_turn.speech.utterance - role = session.current_turn.speech.role - else: - utterance = f"find the {searchable_object}" - role = SpeakerRole.agent - - session.current_state.utterance_queue.append_to_head( - SimBotQueueUtterance(utterance=utterance, role=role), - ) - queue_elem = SimBotQueueUtterance( - utterance=f"go to the {ROOM_SYNONYNMS[room]}", - role=SpeakerRole.agent, - ) - session.current_state.utterance_queue.append_to_head(queue_elem) - session.current_turn.intent.verbal_interaction = SimBotIntent( - type=SimBotIntentType.confirm_before_plan, entity=room - ) - return [] - def _create_goto_room_action(self, room: str) -> SimBotAction: """Create action for going to a room.""" return SimBotAction( diff --git a/src/emma_experience_hub/parsers/simbot/previous_action.py b/src/emma_experience_hub/parsers/simbot/previous_action.py index 07779ad8..4866525c 100644 --- a/src/emma_experience_hub/parsers/simbot/previous_action.py +++ b/src/emma_experience_hub/parsers/simbot/previous_action.py @@ -14,10 +14,7 @@ class SimBotPreviousActionParser(Parser[SimBotSession, SimBotAction]): - """Get the previous interation action from the session for the current turn. - - This parser is most useful when handling approved confirmation requests. - """ + """Get the previous interation action from the session for the current turn.""" def __call__(self, session: SimBotSession) -> SimBotAction: """Get the action from the previous turn.""" diff --git a/src/emma_experience_hub/pipelines/simbot/agent_intent_selection.py b/src/emma_experience_hub/pipelines/simbot/agent_intent_selection.py index 175c1363..f0b68059 100644 --- a/src/emma_experience_hub/pipelines/simbot/agent_intent_selection.py +++ b/src/emma_experience_hub/pipelines/simbot/agent_intent_selection.py @@ -9,7 +9,6 @@ ) from emma_experience_hub.datamodels.simbot import ( SimBotAgentIntents, - SimBotAnyUserIntentType, SimBotIntent, SimBotIntentType, SimBotNLUIntentType, @@ -19,8 +18,6 @@ from emma_experience_hub.functions.simbot.agent_intent_selection import ( SimBotActHandler, SimBotClarificationHandler, - SimBotConfirmationHandler, - set_find_object_in_progress_intent, ) from emma_experience_hub.parsers import NeuralParser from emma_experience_hub.pipelines.simbot.environment_error_catching import ( @@ -28,6 +25,29 @@ ) +def set_find_object_in_progress_intent(session: SimBotSession) -> SimBotAgentIntents: + """Set the intent when find is in progress.""" + if not session.previous_turn or session.previous_turn.intent.physical_interaction is None: + return SimBotAgentIntents(SimBotIntent(type=SimBotIntentType.search)) + + entity = session.previous_turn.intent.physical_interaction.entity + previous_intent = session.previous_turn.intent + # Retain the information that the search was triggered by an act_no_match or act_missing_inventory + is_searching_inferred_object = ( + previous_intent.is_searching_inferred_object + and not session.previous_turn.is_going_to_found_object_from_search + ) + if is_searching_inferred_object and previous_intent.verbal_interaction is not None: + return SimBotAgentIntents( + SimBotIntent(type=SimBotIntentType.search, entity=entity), + SimBotIntent( + type=previous_intent.verbal_interaction.type, + entity=entity, + ), + ) + return SimBotAgentIntents(SimBotIntent(type=SimBotIntentType.search, entity=entity)) + + class SimBotAgentIntentSelectionPipeline: """Determine HOW the agent should act given all the information. @@ -43,7 +63,6 @@ def __init__( action_predictor_client: SimbotActionPredictionClient, environment_error_pipeline: SimBotEnvironmentErrorCatchingPipeline, _enable_clarification_questions: bool = True, - _enable_confirmation_questions: bool = True, _enable_search_actions: bool = True, _enable_search_after_no_match: bool = True, _enable_search_after_missing_inventory: bool = True, @@ -51,14 +70,12 @@ def __init__( ) -> None: self.clarification_handler = SimBotClarificationHandler() self._features_client = features_client - self.confirmation_handler = SimBotConfirmationHandler(_enable_search_after_no_match) self.act_handler = SimBotActHandler( features_client=features_client, nlu_intent_client=nlu_intent_client, nlu_intent_parser=nlu_intent_parser, action_predictor_client=action_predictor_client, _enable_clarification_questions=_enable_clarification_questions, - _enable_confirmation_questions=_enable_confirmation_questions, _enable_search_actions=_enable_search_actions, _enable_search_after_no_match=_enable_search_after_no_match, _enable_search_after_missing_inventory=_enable_search_after_missing_inventory, @@ -137,14 +154,6 @@ def handle_clarification_intent(self, session: SimBotSession) -> Optional[SimBot """Select the action intent when the user intent is responding to a clarification.""" return self.clarification_handler.run(session) - def handle_confimation_intent(self, session: SimBotSession) -> Optional[SimBotAgentIntents]: - """Select the action intent when the user intent is responding to a confirmation.""" - intents = self.confirmation_handler.run(session) - # If the confirmation handler did not set the intent, use the act handler - if intents is None: - return self.handle_act_intent(session) - return intents - def _get_user_intent_handler( self, user_intent: SimBotUserIntentType ) -> Callable[[SimBotSession], Optional[SimBotAgentIntents]]: @@ -152,8 +161,6 @@ def _get_user_intent_handler( switcher = { SimBotIntentType.act: self.handle_act_intent, SimBotIntentType.clarify_answer: self.handle_clarification_intent, - SimBotIntentType.confirm_yes: self.handle_confimation_intent, - SimBotIntentType.confirm_no: self.handle_confimation_intent, } return switcher[user_intent] @@ -178,7 +185,7 @@ def _used_lightweight_dialog_with_stop_token(self, session: SimBotSession) -> bo return previous_action_was_end_of_trajectory and previous_dialog_was_lightweight - def _should_skip_action_selection(self, user_intent_type: SimBotAnyUserIntentType) -> bool: + def _should_skip_action_selection(self, user_intent_type: SimBotUserIntentType) -> bool: """No action needed after an invalid utterance, or an environment error.""" return user_intent_type.is_invalid_user_utterance @@ -191,9 +198,4 @@ def _set_intent_from_environment(self, session: SimBotSession) -> Optional[SimBo session.current_state.utterance_queue.reset() session.current_state.find_queue.reset() return SimBotAgentIntents() - # If it is an unsupported navigation that we can handle, we need to confirm before acting - if self._environment_error_pipeline.is_handling_unsupported_navigation(session): - return SimBotAgentIntents( - verbal_interaction=SimBotIntent(type=SimBotIntentType.confirm_before_plan) - ) return None diff --git a/src/emma_experience_hub/pipelines/simbot/user_intent_extraction.py b/src/emma_experience_hub/pipelines/simbot/user_intent_extraction.py index e03c255a..c5a519ba 100644 --- a/src/emma_experience_hub/pipelines/simbot/user_intent_extraction.py +++ b/src/emma_experience_hub/pipelines/simbot/user_intent_extraction.py @@ -3,7 +3,6 @@ from loguru import logger -from emma_experience_hub.api.clients import ConfirmationResponseClassifierClient from emma_experience_hub.datamodels.simbot import ( SimBotIntentType, SimBotSession, @@ -22,13 +21,8 @@ class SimBotUserIntentExtractionPipeline: def __init__( self, - confirmation_response_classifier: ConfirmationResponseClassifierClient, - _enable_confirmation_questions: bool = True, _enable_clarification_questions: bool = True, ) -> None: - self._confirmation_response_classifier = confirmation_response_classifier - - self._enable_confirmation_questions = _enable_confirmation_questions self._enable_clarification_questions = _enable_clarification_questions def run(self, session: SimBotSession) -> Optional[SimBotUserIntentType]: @@ -66,10 +60,6 @@ def handle_response_to_question(self, session: SimBotSession) -> SimBotUserInten if verbal_interaction_intent_type.triggers_disambiguation_question: return self.handle_clarification_response() - # Did agent ask for confirmation? - if verbal_interaction_intent_type.triggers_confirmation_question: - return self.handle_confirmation_request_approval(session) - raise NotImplementedError("There is no known way to handle the type of question provided.") def handle_clarification_response(self) -> SimBotUserIntentType: @@ -80,39 +70,11 @@ def handle_clarification_response(self) -> SimBotUserIntentType: # Assume it's a clarification answerr return SimBotIntentType.clarify_answer - def handle_confirmation_request_approval(self, session: SimBotSession) -> SimBotUserIntentType: - """Check if the confirmation request was approved and return the correct intent.""" - # Make sure the user utterance exists - if not session.current_turn.speech: - raise AssertionError("There is no utterance from the user? That's not right") - utterance = session.current_turn.speech.utterance.lower() - - # Make sure the agent question exists - if session.previous_turn is None or session.previous_turn.actions.dialog is None: - raise AssertionError("There is no question from the agent? That's not right") - previous_dialog_action = session.previous_turn.actions.dialog - previous_agent_utterance = previous_dialog_action.payload.value.lower() - - # Run the confirmation classifier - confirmation_approved = self._confirmation_response_classifier.is_request_approved( - " ".join([previous_agent_utterance, utterance]).lower() - ) - # If the utterance is NOT a response to the confirmation request - if confirmation_approved is None: - raise AssertionError("Utterance is not a response to the confirmation request") - - if confirmation_approved: - logger.debug("Utterance approves of confirmation request.") - return SimBotIntentType.confirm_yes - - logger.debug("Utterance denies confirmation request.") - return SimBotIntentType.confirm_no - def _was_question_asked_in_previous_turn( self, previous_turn: Optional[SimBotSessionTurn] ) -> bool: """Was a question asked by the agent in the previous turn?""" - if not (self._enable_clarification_questions or self._enable_confirmation_questions): + if not self._enable_clarification_questions: return False if self._was_previous_action_unsuccessful(previous_turn): return False