Conversation
213289d to
e942df4
Compare
e942df4 to
f652d81
Compare
da5ddb0 to
66ad9cf
Compare
e3af211 to
9aae64d
Compare
| self.result_processor = result_processor | ||
| self.num_samples = config.num_samples | ||
| self.save_intermediate_results = config.save_intermediate_results | ||
| self.dataloader = HFDataloader() |
There was a problem hiding this comment.
Would the idea be that we're keeping this hard coded atm and only once new dataloaders would be needed we refactor such that you can inject a dataloader into the ResponseGenerator?
There was a problem hiding this comment.
I'm wondering about the requirements: is it that a task can be loaded from HF as well as from some other source by using the same path (just by switching a dataloader) or that certain tasks are to be loaded from HF and some other tasks from some other source?
| @@ -224,7 +225,7 @@ def _check_no_duplicate_names(cls) -> None: | |||
|
|
|||
| def make_sure_all_hf_datasets_are_in_cache() -> None: | |||
There was a problem hiding this comment.
Was not part of this PR, but I realized this function is only used in the ci. Maybe a comment about that would be adequate. wdyt?
|
I really like the abstraction of the dataloader. So far lgtm overall. |
PR Checklist
/docs/).What type of PR is this? (check all applicable)
Description
Move the data loading out of the BaseTask into a Dataloader class. This would make it possible to add additional Dataloader classes to load datasets from other sources.
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Please replace this line with instructions on how to test your changes, a note
on the hardware and config this has been tested on, as well as any relevant
additional information.
Added/updated tests?
have not been included
[optional] Are there any post deployment tasks we need to perform?