-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Pending Release] Updating to Mephisto 0.4.0 #3982
Conversation
I checked out the Mephisto
|
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.
Thanks for making this change! One comment: I'd also change the README at https://github.com/facebookresearch/ParlAI/blob/master/parlai/crowdsourcing/README.md to reflect that now the YAML files are in hydra_configs/conf/
. In particular, can you still do conf=example
on the command line to specify the hydra_configs/conf/example.yaml
file, or do you have to pass in a --config-dir
arg?
Yes the behavior remains the same, hydra just uses the provided directory as the root. Can edit the README. Basically the new hydra requirements is that we always specify a |
@@ -10,7 +10,7 @@ Tasks are launched by calling the appropriate run script: for instance, an ACUTE | |||
|
|||
### Specifying your own YAML file | |||
|
|||
The easiest way to specify a different YAML file is to create a new file, say, `my_params.yaml`, in the `conf/` subfolder of the task. Then, you can launch HITs with `python ${TASK_FOLDER}/run.py conf=my_params`. | |||
The easiest way to specify a different YAML file is to create a new file, say, `my_params.yaml`, in the `hydra_configs/conf/` subfolder of the task. Then, you can launch HITs with `python ${TASK_FOLDER}/run.py conf=my_params`. |
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.
Thanks for the change :) The conf/
mention on line 9 should also be changed to hydra_configs/conf/
, right?
Patch description
Updates all of the scripts using Mephisto to be compliant with the Hydra 1.1 version that Mephisto
0.4.0
uses. Also updates the crowdsourcing test script.NOTE
Mephisto
v0.4.0
is not yet released, this branch is setting us up for after a few in-flight PRs land to cleanly upgrade everyone at once.v0.4.0
will also be the first version on PyPI, if that is helpful. Happy to make changes on how requirements are pulled given that information.Testing steps
TBD - I launched a few of the scripts and found they didn't throw the
@Package directive missing
error that many were seeing before.