Skip to content
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 - Handle episode lists instead of ranges. #2026

Merged
merged 3 commits into from
Aug 21, 2024
Merged

Conversation

0mdc
Copy link
Contributor

@0mdc 0mdc commented Aug 20, 2024

Motivation and Context

In previous iterations, the HITL server accepted episode index ranges in the format episodes: 100-104.

This approach had multiple issues:

  • In a crowdsource context, it was not possible to "recycle" individual failed episodes. Entire ranges had to be discarded.
  • If an error occurred during the middle of a session, data from the completed episodes was discarded.
  • Special care was required to sequence episode sets:
    • Tutorial episodes needed to be copied every N indices.
    • Scenes required to be sequenced in order.
  • Once an application was deployed, it was not possible to adjust task distribution.
  • Ambiguous range boundaries causing human errors.

This changeset changes the episode format to accept ranges instead, in the format episodes: 100,101,102,103

How Has This Been Tested

Tested on distributed HITL application.

Types of changes

  • [Development]

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation if required.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes if required.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Aug 20, 2024
Copy link
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

See comments.

examples/hitl/rearrange_v2/app_state_start_session.py Outdated Show resolved Hide resolved
print("Users did not request episodes. Cancelling session.")
return None
# Parse episode string.
episodes_split = episodes_str.split(",")
Copy link
Contributor

Choose a reason for hiding this comment

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

Beware you are kinda duplicating the episode_filter logic in hitl_driver.py. Consider reusing that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a planned refactor post single-learn.

Lots of the code in the rearrange_v2 can be moved to habitat-hitl.

Copy link
Contributor

@eundersander eundersander left a comment

Choose a reason for hiding this comment

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

LGTM!

@0mdc 0mdc merged commit b3ab3d8 into main Aug 21, 2024
3 of 4 checks passed
@0mdc 0mdc deleted the 0mdc/hitl_episode_lists branch August 21, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants