Add Screening Example #883

Etesam913 · 2022-08-03T21:35:34Z

Overview

This pr configures the option to run screening units on `static_react_task`.

Created a new screening_example.yaml configuration
Fixed many bugs with adding screening units to SharedStaticTaskState
Added video and extra information to the documentation page on Screening Units

Mnist Video (this is the showcase in the documentation page below)

mnist-screening-example.mp4

Static React Task Video

screening_demo_example.mp4

New Documentation Page for Screening Unit

codecov-commenter · 2022-08-03T21:42:57Z

Codecov Report

Merging #883 (558079b) into main (1470dd5) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #883      +/-   ##
==========================================
+ Coverage   64.61%   64.64%   +0.02%     
==========================================
  Files         108      108              
  Lines        9313     9312       -1     
==========================================
+ Hits         6018     6020       +2     
+ Misses       3295     3292       -3

Impacted Files	Coverage Δ
mephisto/operations/worker_pool.py	`80.79% <ø> (ø)`
mephisto/abstractions/blueprint.py	`81.88% <100.00%> (+0.28%)`	⬆️
...lueprints/abstract/static_task/static_blueprint.py	`41.48% <100.00%> (+0.67%)`	⬆️
...nts/remote_procedure/remote_procedure_blueprint.py	`42.70% <100.00%> (+0.60%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

🎨 Improved button styling on onboarding page

Etesam913 · 2022-08-04T21:05:35Z

What are your thoughts on putting a video in a documentation page? I compressed down the video, so it is about 9.4mb in size right now.

I do think that it would help a newcomer quickly understand how screening units work.

Etesam913 · 2022-08-05T14:48:57Z

I updated the video

Etesam913 · 2022-08-05T14:55:01Z

docs/web/package.json

@@ -25,6 +25,7 @@
 "prism-react-renderer": "^1.2.1",
 "react": "^17.0.1",
 "react-dom": "^17.0.1",
+ "react-player": "^2.10.1",


This is for being able to play videos in mdx file.

Etesam913 · 2022-08-05T14:57:12Z

examples/static_react_task/run_task.py

+ is_using_screening_units = cfg.mephisto.blueprint["use_screening_task"]
+ shared_state = None
+ if not is_using_screening_units:
+ shared_state = SharedStaticTaskState(
+ static_task_data=[
+ {"text": "This text is good text!"},
+ {"text": "This text is bad text!"},
+ ],
+ validate_onboarding=onboarding_always_valid,
+ )
+
+ else:
+ """
+ When using screening units there has to be a
+ few more properties set on shared_state
+ """
+ shared_state = SharedStaticTaskState(
+ static_task_data=[
+ {"text": "This text is good text!"},
+ {"text": "This text is bad text!"},
+ ],
+ validate_onboarding=onboarding_always_valid,
+ on_unit_submitted=ScreenTaskRequired.create_validation_function(
+ cfg.mephisto,
+ validate_screening_unit,
+ ),
+ screening_data_factory=my_screening_unit_generator(),
+ )
+ shared_state.qualifications += ScreenTaskRequired.get_mixin_qualifications(
+ cfg.mephisto, shared_state
+ )


If you are using the screening unit conf they are more properties that you need to add to your SharedStaticTaskState.

You can add these dynamically to the SharedStaticTaskState after the case:

share_state = ... if is_using_screening_units: shared_state.on_unit_submitted = ... ...

To reduce duplication a bit

Etesam913 · 2022-08-05T14:57:34Z

examples/static_react_task/webapp/src/components/core_components.jsx

+ <div
+ style={{
+ width: "100%",
+ padding: "1.5rem 0",
+ display: "flex",
+ justifyContent: "center",
+ }}
 >
- Move to main task.
- </button>
+ <button
+ className="button is-link"
+ onClick={() => onSubmit({ success: true })}
+ >
+ Move to Main Task
+ </button>
+ </div>


I was peeved by the fact that the onboarding button was not centered on the page.

Yeeeeaaah this could've been adjusted long ago hahaha

JackUrb

I love the new docs page and examples! Only a tiny nit on code duplication, but otherwise I'm excited to have solid examples of this workflow now, and for it to be wrapped into the two most commonly used blueprints.

JackUrb · 2022-08-05T18:27:50Z

mephisto/abstractions/blueprint.py

@@ -8,6 +8,7 @@
 import csv
 from genericpath import exists
 import os
+from rich import print


Unneeded import

JackUrb · 2022-08-05T18:29:05Z

mephisto/operations/worker_pool.py

@@ -470,6 +470,7 @@ async def register_agent(
 function="get_valid_units_for_worker"
 ).time():
 units = task_run.get_valid_units_for_worker(worker)
+


Odd whitespace changes in this file

pringshia · 2022-08-05T18:34:51Z

Fixed many bugs with adding screening units to SharedStaticTaskState

Curious to hear more about this, but thanks for fixing!

Etesam913 · 2022-08-05T18:37:48Z

There were some bugs with hydra conf variables not existing. The changes made to mephisto/abstractions/blueprint.py fixed this. @JackUrb knows more about this.

JackUrb · 2022-08-05T18:45:26Z

Fixed many bugs with adding screening units to SharedStaticTaskState

Curious to hear more about this, but thanks for fixing!

This was my bad - I had made a mistake in the Mephisto magic that extracts the BlueprintMixins from Blueprints that inherit them to run initialization and other methods.

The issue: I wanted to be able to only take the terminal class of a BlueprintMixin. Say someone defines X and someone else makes YExtendsX and your Blueprint uses YExtendsX, when we collect subclasses via mro() we want to call the init of YExtendsX and not that of X. The way I did this before was amass the set of all subclasses of the Blueprint, then drop any class from that set that was a superclass of any other class in there. Unfortunately, when I amassed this list before, it didn't distinguish between Blueprints and BlueprintMixins explicitly. Thus, since static blueprints extend the abstract StaticBlueprint, the resolver would do the following (abridged):

mro() -> [StaticReactBlueprint, StaticBlueprint, OnboardingRequired, ScreenTaskRequired, Blueprint, BlueprintMixin]
Remove self and base classes -> [StaticBlueprint, OnboardingRequired, ScreenTaskRequired]
Remove subclasses of any other class in the set -> [StaticBlueprint]

The fix was to do a step 2.5 that drops any subclasses of Blueprint from the set first.

pringshia

Pretty nice! Thanks for improving the screening developer experience. It's an often-used feature of Mephisto and helps greatly in assuring data quality.

pringshia · 2022-08-05T18:36:42Z

examples/remote_procedure/mnist/run_task.py



 @task_script(default_config_file="launch_with_local")
 def main(operator: Operator, cfg: DictConfig) -> None:
- tasks: List[Dict[str, Any]] = [{}] * cfg.num_tasks
+ tasks: List[Dict[str, Any]] = [{"isScreeningUnit": False}] * cfg.num_tasks


this could also be reverted back to what it was (no property aka undefined) and the frontend would still work since it's doing a truthy boolean check, right?

It's being used like this: const NUM_ANNOTATIONS = taskData.isScreeningUnit ? 1 : 3;

pringshia · 2022-08-05T18:45:40Z

examples/remote_procedure/mnist/run_task.py

+ )
+ shared_state.qualifications += ScreenTaskRequired.get_mixin_qualifications(
+ cfg.mephisto, shared_state
+ )


this is unrelated to this PR, but it seems that ScreenTaskRequired.get_mixin_qualifications is not even using shared_state despite requiring it in the signature.

get_mixin_requirements has a shared signature for all BlueprintMixins, as ideally this will be replaced with <>Blueprint.get_mixin_qualifications automatically pulling required qualifications into the state under the hood.

pringshia · 2022-08-05T18:47:22Z

examples/remote_procedure/mnist/run_task.py

+ shared_state = SharedRemoteProcedureTaskState(
+ static_task_data=tasks,
+ function_registry=function_registry,
+ )


this could also be condensed based on Jack's earlier comment

✨ Add ScreeningArgs to SharedStaticTaskState

a39eed5

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 Aug 3, 2022

Etesam913 added 8 commits August 4, 2022 10:24

🐛 Bug with screening unit only loading

e6c789a

🐛 Fixed bug with validator function not running

9b7895c

🔧 Configured custom screening_data_factory

eb41c61

📝 Fixed typo in onboarding_example.yaml

54e7dd0

🐛 Fixed bug with qualified worker stuck in unit

bd8996e

🔧 Configured run_task.py to work on all conf's

8305eeb

🎨 Improved button styling on onboarding page

📝 Rewrote the screening units docs page

654c356

📝 Added some bullet points below showcase vid

6f224e7

🔥 Removed static video to use github cdn video

2b7638c

Etesam913 commented Aug 5, 2022

View reviewed changes

➖ Removed rich from worker_pool.py as it is unused

53c9cef

Etesam913 changed the title ~~[WIP] Add Screening Example~~ Add Screening Example Aug 5, 2022

Etesam913 requested review from JackUrb and pringshia and removed request for JackUrb August 5, 2022 15:00

✨ Added screening unit config to mnist

62449f9

JackUrb approved these changes Aug 5, 2022

View reviewed changes

pringshia approved these changes Aug 5, 2022

View reviewed changes

♻️ Refactored run_task.py for screening

b5be62a

📝 Updated screening units docs with mnist example

558079b

Etesam913 merged commit 8e84e2a into main Aug 5, 2022

Etesam913 deleted the add-screening-example branch August 5, 2022 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Screening Example #883

Add Screening Example #883

Etesam913 commented Aug 3, 2022 •

edited

Loading

codecov-commenter commented Aug 3, 2022 •

edited

Loading

Etesam913 commented Aug 4, 2022 •

edited

Loading

Etesam913 commented Aug 5, 2022

Etesam913 Aug 5, 2022

Etesam913 Aug 5, 2022

JackUrb Aug 5, 2022

Etesam913 Aug 5, 2022

JackUrb Aug 5, 2022

JackUrb left a comment

JackUrb Aug 5, 2022

JackUrb Aug 5, 2022

pringshia commented Aug 5, 2022

Etesam913 commented Aug 5, 2022

JackUrb commented Aug 5, 2022 •

edited

Loading

pringshia left a comment

pringshia Aug 5, 2022 •

edited

Loading

Etesam913 Aug 5, 2022

pringshia Aug 5, 2022

JackUrb Aug 5, 2022

pringshia Aug 5, 2022

Add Screening Example #883

Add Screening Example #883

Conversation

Etesam913 commented Aug 3, 2022 • edited Loading

Overview

This pr configures the option to run screening units on static_react_task.

Mnist Video (this is the showcase in the documentation page below)

Static React Task Video

New Documentation Page for Screening Unit

codecov-commenter commented Aug 3, 2022 • edited Loading

Codecov Report

Etesam913 commented Aug 4, 2022 • edited Loading

Etesam913 commented Aug 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JackUrb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pringshia commented Aug 5, 2022

Etesam913 commented Aug 5, 2022

JackUrb commented Aug 5, 2022 • edited Loading

pringshia left a comment

Choose a reason for hiding this comment

pringshia Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etesam913 commented Aug 3, 2022 •

edited

Loading

This pr configures the option to run screening units on `static_react_task`.

codecov-commenter commented Aug 3, 2022 •

edited

Loading

Etesam913 commented Aug 4, 2022 •

edited

Loading

JackUrb commented Aug 5, 2022 •

edited

Loading

pringshia Aug 5, 2022 •

edited

Loading