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

Task config #289

Merged
merged 10 commits into from
Sep 13, 2024
Merged

Task config #289

merged 10 commits into from
Sep 13, 2024

Conversation

hynky1999
Copy link
Collaborator

What does this implement/fix? Explain your changes.

  1. This PR adds new adds following new args to task config
  • hf_revision (allows locking to certain commit)
  • hf_filter (allows filtering the datasets)
  1. Set defaults as early as possible for lighteval task config
  2. Removing of unused args from task class:
      self.save_queries: bool = False
      self.logfile_name: Optional[Path] = None
      self.is_main_process: bool = False
  1. Improved typings

  2. I noticed that following args are unused, maybe we should create issue to implement that functionality or remove it ?

    frozen: bool = False
    output_regex: Optional[str] = None

@hynky1999 hynky1999 requested a review from NathanHB September 5, 2024 09:49
dataset = dataset.filter(dataset_filter)

# It returns DatasetDict because we don't specify a split
return dataset # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

why do we ignore the type here ?

Copy link
Collaborator Author

@hynky1999 hynky1999 Sep 13, 2024

Choose a reason for hiding this comment

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

Because it doesn't have correct type
load_dataset returns Dataset | DatasetDict | IterableDataset | IterableDatasetDict, based on the input args and afaik there is unspecified contract that if the we don't provide streaming and split arg we get DatasetDict. However there is no way to achieve this on typings level, so I just ignore this error.

If the question was why I put there the type: ignore it's because even tho we don't have a typechecker in the quality checks, I do have it on in my vscode (pyright) and it shows red when there is a typing problem.

@NathanHB
Copy link
Member

Looks great !
the output_regex is legacy from the harness, we needed it for bigbench tasks but this is now handled by the metrics direclty. Frozen was to make sure the task did not change over time but we are now using task versioning for this.

@hynky1999 hynky1999 merged commit 919be47 into main Sep 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants