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

Adding reasonable defaults to our examples for max concurrent units #747

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

JackUrb
Copy link
Contributor

@JackUrb JackUrb commented Apr 4, 2022

Overview

Mostly just the title. As brought up in #746, not having these kinds of defaults means that if people use the default values they're more likely to hit a bad experience (too much server load) rather than be low utilization. To make the basic case better, I've selected some defaults that should be pretty good on this tradeoff (at least until we hit the point that we can use our metrics tooling to scale this dynamically... in version 2.0 or something).

@JackUrb JackUrb requested a review from pringshia April 4, 2022 21:03
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 4, 2022
@@ -8,3 +8,6 @@ mephisto:
world_file: ${task_dir}/demo_worlds.py
task_description_file: ${task_dir}/task_description.html
num_conversations: 1
task:
# We expect to be able to handle 25 concurrent conversations without issue
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a typo? Says 25 in the comment but 50 for the value...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

25 conversations, but they require 2 connected people per. I see the comment is misleading here, can make explicit

@@ -8,3 +8,6 @@ mephisto:
world_file: ${task_dir}/demo_worlds.py
task_description_file: ${task_dir}/task_description.html
num_conversations: 1
task:
# We expect to be able to handle 25 concurrent conversations without issue
max_num_concurrent_units: 50
Copy link
Contributor

Choose a reason for hiding this comment

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

50 here^

@JackUrb JackUrb merged commit 124458c into main Apr 8, 2022
@JackUrb JackUrb deleted the reasonable-default-max-concurrent branch April 8, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants