-
Notifications
You must be signed in to change notification settings - Fork 510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HITL - Rearrange State Machine #1964
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my context.
|
||
from habitat_hitl.app_states.app_service import AppService | ||
|
||
START_SESSION_DELAY: Final[float] = 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment to explain the empirically chosen value? I see the comment below about disconnects, but still curious.
user_index_to_agent_index_map: Dict[int, int] = {} | ||
for user_index in range(len(self._user_data)): | ||
user_index_to_agent_index_map[user_index] = self._user_data[ | ||
user_index | ||
].gui_agent_controller._agent_idx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirming that user-index is networked humans who have joined and != agent-count/index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct! This is the "decoupling" work I still have to do.
return all( | ||
self._user_data[user_index].episode_finished | ||
for user_index in self._users.indices(Mask.ALL) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interested to see how can I expose user-data for llm-agent to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not ideal, but the easiest solution would be to query the agent controller types at initialization and handle things from there.
task_instruction = current_episode.instruction | ||
# TODO: Users will have different instructions. | ||
for user_index in self._users.indices(Mask.ALL): | ||
self._user_data[user_index].task_instruction = task_instruction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I see. I am setting such metadata/data for llm-agent directly from habitat Env
class. It'd be good to standardize all data-setting to flow from here.
Therefore, interested to see how can I consume this user-data
variable at init/reset time. We can just pass this on to ctrl_helper
, right? It'll just be ignore as part of *args, **kwargs
for other controllers which have no use for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be doable.
If you only need user events, you should be able to bind events from rearrange_v2.on_environment_reset()
as you will have access to both variables. I'll be adding them in the next 2 PRs.
if self._user_data[user_index].show_gui_text: | ||
controls_str += "H: Toggle help\n" | ||
controls_str += "Look: Middle click (drag), I, K\n" | ||
controls_str += "Walk: W, S\n" | ||
controls_str += "Turn: A, D\n" | ||
controls_str += "Finish episode: Zero (0)\n" | ||
controls_str += "Open/close: Double-click\n" | ||
controls_str += "Pick object: Double-click\n" | ||
controls_str += "Place object: Right click (hold)\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an option to see current-instruction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's currently displayed via the status message.
client_helper = self._app_service.remote_client_state._client_helper | ||
idle_time = client_helper.get_idle_time(user_index) | ||
if idle_time > 10: | ||
controls_str += f"Idle time {idle_time}s\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note if you kick users/agents for idle_time > SOME_THRESH
this may need to be tweaked for LLM-agent versus human user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LLM agent will not have an idle counter. I still have to decouple this.
idle_time = client_helper.get_idle_time(user_index) | ||
if idle_time > 10: | ||
controls_str += f"Idle time {idle_time}s\n" | ||
|
||
return controls_str | ||
|
||
def _get_status_text(self, user_index: int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh! Lovely. I can pass current_high_level_action
being executed by LLM-agent here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If time permits, I can also add a text
field to viewports so that its rendered into the picture-in-picture view.
def _is_any_user_active(self) -> bool: | ||
return any( | ||
self._user_data[user_index].gui_input.get_any_input() | ||
for user_index in range(self._app_data.max_user_count) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work for LLM-agent? Or do you not care? We probably should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work without changes with LLM agents. user_data
, max_user_count
and user_index
all relate to human users.
In the LLM case, max_user_count
will be 1
, but agent_count
will be 2
. I have to decouple these concepts within user_data
(will most likely create an analogous agent_data
struct).
* Add viewports. * Add viewports to rearrange_v2. * Add object visiblity and viewport visbility layers. * Hide self in viewports. * Don't render viewports if networking is disabled. * Turn rearrange_v2 into a state machine. Add lobby. * Disable rearrange_v2 test. * Config changes. * Change episode termination states. * Add comment to lobby start session delay.
* Add viewports. * Add viewports to rearrange_v2. * Add object visiblity and viewport visbility layers. * Hide self in viewports. * Don't render viewports if networking is disabled. * Turn rearrange_v2 into a state machine. Add lobby. * Disable rearrange_v2 test. * Config changes. * Change episode termination states. * Add comment to lobby start session delay.
* Add viewports. * Add viewports to rearrange_v2. * Add object visiblity and viewport visbility layers. * Hide self in viewports. * Don't render viewports if networking is disabled. * Turn rearrange_v2 into a state machine. Add lobby. * Disable rearrange_v2 test. * Config changes. * Change episode termination states. * Add comment to lobby start session delay.
* Add viewports. * Add viewports to rearrange_v2. * Add object visiblity and viewport visbility layers. * Hide self in viewports. * Don't render viewports if networking is disabled. * Turn rearrange_v2 into a state machine. Add lobby. * Disable rearrange_v2 test. * Config changes. * Change episode termination states. * Add comment to lobby start session delay.
Motivation and Context
This changeset turns
rearrange_v2
into a state machine with the following states:lobby
.max_client_count
users are connected, changes state torearrange_v2
.rearrange_v2
application.reset
.state_machine-2024-05-17_13.15.44.mp4
Notes
main.py
instead ofrearrange_v2.py
.rearrange_v2.py
will be renamed later to simplify merging of concurrent changes.Depends on:
Limitations
ClientHelper
depends on connection events, its lifetime is now coupled withRemoteClientState
. This will be refactored in a later pass.rearrange_v2
smoke test is disabled until it is stabilized.How Has This Been Tested
Tested on multiplayer HITL application.
Types of changes
Checklist