-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Warning on unrecommended ACUTE-Eval value #3536
Conversation
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.
Generally looks good to me. Suggestion on slight wording change.
warn_once( | ||
'It is *strongly* recommended to use a ' | ||
'args.task.maximum_units_per_worker value of 1, as was done in the ' | ||
'ACUTE-Eval paper! Go to GitHub Discussions for more information.' |
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.
A slight change suggestion:
'It is *strongly* recommended to use a '
'args.task.maximum_units_per_worker value of 1, as was done in the '
'ACUTE-Eval paper! Anything else will not provide consistent results as '
'quoted there. Go to ParlAI's GitHub Discussion page '
'if you'd like to discuss further.
I think the current wording implies we already have a discussion on it there.
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.
Good point, changed
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 doing! Dependabot fixed the cleaninstall test in master.
from omegaconf import MISSING, DictConfig | ||
|
||
from parlai.crowdsourcing.tasks.acute_eval.acute_eval_agent_state import ( | ||
AcuteEvalAgentState, | ||
) | ||
from parlai.crowdsourcing.tasks.acute_eval.acute_eval_builder import AcuteEvalBuilder | ||
from parlai.crowdsourcing.tasks.acute_eval.acute_eval_runner import AcuteEvalRunner | ||
from parlai.utils.misc import warn_once | ||
|
||
if TYPE_CHECKING: |
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.
this is super cool!
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.
Patch description
Print a big ugly warning message if the user tries to set
mephisto.task.maximum_units_per_worker
to anything other than the recommended value of 0Testing steps