From 55ab4d54ac10d7b90e64a9ea78fc81c6cfdde7fe Mon Sep 17 00:00:00 2001 From: Jack Urbanek Date: Thu, 3 Nov 2022 15:59:29 -0400 Subject: [PATCH 1/4] Adding gold units and functionality --- .../abstract/static_task/static_blueprint.py | 11 ++++++++--- .../blueprints/mixins/screen_task_required.py | 4 +++- .../blueprints/mixins/use_gold_unit.py | 17 +++++++++++++++++ .../remote_procedure_blueprint.py | 15 +++++++++++---- mephisto/operations/client_io_handler.py | 1 + 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/mephisto/abstractions/blueprints/abstract/static_task/static_blueprint.py b/mephisto/abstractions/blueprints/abstract/static_task/static_blueprint.py index d45c7e533..777b2dc29 100644 --- a/mephisto/abstractions/blueprints/abstract/static_task/static_blueprint.py +++ b/mephisto/abstractions/blueprints/abstract/static_task/static_blueprint.py @@ -21,6 +21,11 @@ ScreenTaskRequiredArgs, ScreenTaskSharedState, ) +from mephisto.abstractions.blueprints.mixins.use_gold_unit import ( + UseGoldUnit, + UseGoldUnitArgs, + GoldUnitSharedState, +) from mephisto.data_model.assignment import InitializationData from mephisto.abstractions.blueprints.abstract.static_task.static_agent_state import ( StaticAgentState, @@ -53,7 +58,7 @@ @dataclass class SharedStaticTaskState( - ScreenTaskSharedState, OnboardingSharedState, SharedTaskState + ScreenTaskSharedState, OnboardingSharedState, GoldUnitSharedState, SharedTaskState ): static_task_data: Iterable[Any] = field( default_factory=list, @@ -71,7 +76,7 @@ class SharedStaticTaskState( @dataclass class StaticBlueprintArgs( - ScreenTaskRequiredArgs, OnboardingRequiredArgs, BlueprintArgs + ScreenTaskRequiredArgs, OnboardingRequiredArgs, UseGoldUnitArgs, BlueprintArgs ): _blueprint_type: str = BLUEPRINT_TYPE_STATIC _group: str = field( @@ -107,7 +112,7 @@ class StaticBlueprintArgs( ) -class StaticBlueprint(ScreenTaskRequired, OnboardingRequired, Blueprint): +class StaticBlueprint(ScreenTaskRequired, OnboardingRequired, UseGoldUnit, Blueprint): """ Abstract blueprint for a task that runs without any extensive backend. These are generally one-off tasks sending data to the frontend and then diff --git a/mephisto/abstractions/blueprints/mixins/screen_task_required.py b/mephisto/abstractions/blueprints/mixins/screen_task_required.py index c6d534656..bdff65f3b 100644 --- a/mephisto/abstractions/blueprints/mixins/screen_task_required.py +++ b/mephisto/abstractions/blueprints/mixins/screen_task_required.py @@ -118,7 +118,9 @@ def init_screening_config( self.use_screening_task = args.blueprint.get("use_screening_task", False) if not self.use_screening_task: return - + assert ( + args.blueprint.get("use_golds") is not True + ), "Gold units currently cannot be used with screening units" # Runs that are using a qualification task should be able to assign # a specially generated unit to unqualified workers self.passed_qualification_name = args.blueprint.passed_qualification_name diff --git a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py index 73fa011bd..b0a3a0c28 100644 --- a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py +++ b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py @@ -198,6 +198,8 @@ def init_gold_config( self.min_golds = args.blueprint.min_golds self.max_incorrect_golds = args.blueprint.max_incorrect_golds + self.gold_units_launched = 0 + self.gold_unit_cap = args.blueprint.max_gold_units find_or_create_qualification(task_run.db, self.golds_correct_qual_name) find_or_create_qualification(task_run.db, self.golds_failed_qual_name) @@ -213,6 +215,15 @@ def assert_mixin_args(cls, args: "DictConfig", shared_state: "SharedTaskState"): "Can only run this task type with one allowed concurrent unit at a time per worker, to ensure " "golds are completed in order." ) + assert ( + args.blueprint.get("use_screening_task") is not True + ), "Gold units currently cannot be used with screening units" + max_gold_units = args.blueprint.max_gold_units + assert max_gold_units is not None, ( + "You must supply a blueprint.max_gold_units argument to set the maximum number of " + "additional units you will pay out for evaluating on gold units. Note that you " + "do pay for gold units, they are just like any other units." + ) gold_qualification_base = args.blueprint.gold_qualification_base assert ( gold_qualification_base is not None @@ -257,6 +268,9 @@ def should_produce_gold_for_worker(self, worker: "Worker") -> bool: completed_units, correct_units, incorrect_units, self.max_incorrect_golds ): return False + if correct_units >= self.min_golds: + if self.gold_units_launched >= self.gold_unit_cap: + return False # they qualify, but we don't have golds to launch return self.worker_needs_gold( completed_units, correct_units, incorrect_units, self.min_golds ) @@ -278,7 +292,10 @@ def update_qualified_status(self, worker: "Worker") -> bool: def get_gold_unit_data_for_worker( self, worker: "Worker" ) -> Optional[Dict[str, Any]]: + if self.gold_units_launched >= self.gold_unit_cap: + return None try: + self.gold_units_launched += 1 return self.get_gold_for_worker(worker) except Exception as e: logger.warning(f"Could not generate gold for {worker} due to {e}") diff --git a/mephisto/abstractions/blueprints/remote_procedure/remote_procedure_blueprint.py b/mephisto/abstractions/blueprints/remote_procedure/remote_procedure_blueprint.py index 4a9b57dc4..32f64bb05 100644 --- a/mephisto/abstractions/blueprints/remote_procedure/remote_procedure_blueprint.py +++ b/mephisto/abstractions/blueprints/remote_procedure/remote_procedure_blueprint.py @@ -9,17 +9,22 @@ BlueprintArgs, SharedTaskState, ) +from dataclasses import dataclass, field from mephisto.abstractions.blueprints.mixins.onboarding_required import ( OnboardingRequired, OnboardingSharedState, OnboardingRequiredArgs, ) -from dataclasses import dataclass, field from mephisto.abstractions.blueprints.mixins.screen_task_required import ( ScreenTaskRequired, ScreenTaskRequiredArgs, ScreenTaskSharedState, ) +from mephisto.abstractions.blueprints.mixins.use_gold_unit import ( + UseGoldUnit, + UseGoldUnitArgs, + GoldUnitSharedState, +) from mephisto.data_model.assignment import InitializationData from mephisto.abstractions.blueprints.remote_procedure.remote_procedure_agent_state import ( RemoteProcedureAgentState, @@ -59,7 +64,7 @@ @dataclass class SharedRemoteProcedureTaskState( - ScreenTaskSharedState, OnboardingSharedState, SharedTaskState + ScreenTaskSharedState, OnboardingSharedState, GoldUnitSharedState, SharedTaskState ): function_registry: Optional[ Mapping[ @@ -75,7 +80,7 @@ class SharedRemoteProcedureTaskState( @dataclass class RemoteProcedureBlueprintArgs( - ScreenTaskRequiredArgs, OnboardingRequiredArgs, BlueprintArgs + ScreenTaskRequiredArgs, OnboardingRequiredArgs, UseGoldUnitArgs, BlueprintArgs ): _blueprint_type: str = BLUEPRINT_TYPE_REMOTE_PROCEDURE _group: str = field( @@ -114,7 +119,9 @@ class RemoteProcedureBlueprintArgs( @register_mephisto_abstraction() -class RemoteProcedureBlueprint(ScreenTaskRequired, OnboardingRequired, Blueprint): +class RemoteProcedureBlueprint( + ScreenTaskRequired, OnboardingRequired, UseGoldUnit, Blueprint +): """Blueprint for a task that runs a parlai chat""" AgentStateClass: ClassVar[Type["AgentState"]] = RemoteProcedureAgentState diff --git a/mephisto/operations/client_io_handler.py b/mephisto/operations/client_io_handler.py index c69eb9cdc..15adbc393 100644 --- a/mephisto/operations/client_io_handler.py +++ b/mephisto/operations/client_io_handler.py @@ -371,6 +371,7 @@ def enqueue_agent_details(self, request_id: str, additional_data: Dict[str, Any] ) self.process_outgoing_queue(self.message_queue) self.log_metrics_for_packet(self.request_id_to_packet[request_id]) + # TODO Sometimes this request ID is lost, and we don't quite know why del self.request_id_to_channel_id[request_id] del self.request_id_to_packet[request_id] From 80e3365b02aee419da01b1e29d78eff82ec3a0a2 Mon Sep 17 00:00:00 2001 From: Jack Urbanek Date: Thu, 3 Nov 2022 16:01:43 -0400 Subject: [PATCH 2/4] Adding arg --- .../abstractions/blueprints/mixins/use_gold_unit.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py index b0a3a0c28..d9af7736b 100644 --- a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py +++ b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py @@ -73,6 +73,16 @@ class UseGoldUnitArgs: ) }, ) + max_gold_units: int = field( + default=MISSING, + metadata={ + "help": ( + "The maximum number of gold units that can be launched " + "with this batch, specified to limit the number of golds " + "you may need to pay out for." + ) + }, + ) GoldFactory = Callable[["Worker"], Dict[str, Any]] From 2c5337884dd8488c6bd59b9384e6c5728f7b4ded Mon Sep 17 00:00:00 2001 From: Jack Urbanek Date: Thu, 3 Nov 2022 16:17:20 -0400 Subject: [PATCH 3/4] ordering for omegaconf reasons --- .../blueprints/mixins/use_gold_unit.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py index d9af7736b..d30d2d6b0 100644 --- a/mephisto/abstractions/blueprints/mixins/use_gold_unit.py +++ b/mephisto/abstractions/blueprints/mixins/use_gold_unit.py @@ -53,6 +53,16 @@ class UseGoldUnitArgs: "help": ("Basename for a qualification that tracks gold completion rates") }, ) + max_gold_units: int = field( + default=MISSING, + metadata={ + "help": ( + "The maximum number of gold units that can be launched " + "with this batch, specified to limit the number of golds " + "you may need to pay out for." + ) + }, + ) use_golds: bool = field( default=False, metadata={"help": ("Whether or not to use gold tasks in this run.")}, @@ -73,16 +83,6 @@ class UseGoldUnitArgs: ) }, ) - max_gold_units: int = field( - default=MISSING, - metadata={ - "help": ( - "The maximum number of gold units that can be launched " - "with this batch, specified to limit the number of golds " - "you may need to pay out for." - ) - }, - ) GoldFactory = Callable[["Worker"], Dict[str, Any]] From aeb1635ee8a415bef5e7cffe6eef1078d2a494d0 Mon Sep 17 00:00:00 2001 From: Jack Urbanek Date: Fri, 4 Nov 2022 14:47:08 -0400 Subject: [PATCH 4/4] Pin python version --- .github/workflows/cypress-end-to-end-tests.yml | 12 ++++++++++++ .github/workflows/deploy-docs.yml | 2 ++ .github/workflows/npm-check.yml | 2 ++ .github/workflows/pre-commit.yml | 2 ++ .github/workflows/test-deploy-docs.yml | 2 ++ .github/workflows/version-sync.yml | 2 ++ 6 files changed, 22 insertions(+) diff --git a/.github/workflows/cypress-end-to-end-tests.yml b/.github/workflows/cypress-end-to-end-tests.yml index 5816894b4..84c9c3e2f 100644 --- a/.github/workflows/cypress-end-to-end-tests.yml +++ b/.github/workflows/cypress-end-to-end-tests.yml @@ -69,6 +69,8 @@ jobs: - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 @@ -114,6 +116,8 @@ jobs: - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 @@ -159,6 +163,8 @@ jobs: - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 @@ -203,6 +209,8 @@ jobs: - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 @@ -249,6 +257,8 @@ jobs: - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 @@ -293,6 +303,8 @@ jobs: uses: actions/checkout@v2 - name: 🐍 Installing python uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🪨 Setup Node uses: actions/setup-node@v3 diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 0c9414b5c..9a39f2f0b 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -21,6 +21,8 @@ jobs: node-version: 16.16.0 cache: yarn - uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🤖 Install Mephisto run: | cd ../../ diff --git a/.github/workflows/npm-check.yml b/.github/workflows/npm-check.yml index 80602ea99..a42c602b7 100644 --- a/.github/workflows/npm-check.yml +++ b/.github/workflows/npm-check.yml @@ -10,6 +10,8 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: Install Mephisto run: pip install -e . - name: Run version sync script diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index e0214a4fe..e4c09f3d9 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -11,4 +11,6 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 + with: + python-version: 3.8 - uses: pre-commit/action@v2.0.0 diff --git a/.github/workflows/test-deploy-docs.yml b/.github/workflows/test-deploy-docs.yml index 5041b1501..b93d5a0e8 100644 --- a/.github/workflows/test-deploy-docs.yml +++ b/.github/workflows/test-deploy-docs.yml @@ -23,6 +23,8 @@ jobs: node-version: 16.16.0 cache: yarn - uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: 🤖 Install Mephisto run: | diff --git a/.github/workflows/version-sync.yml b/.github/workflows/version-sync.yml index 504118e62..848cd89e5 100644 --- a/.github/workflows/version-sync.yml +++ b/.github/workflows/version-sync.yml @@ -11,6 +11,8 @@ jobs: steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 + with: + python-version: 3.8 - name: Install Mephisto run: pip install -e . - name: Run version sync script