Skip to content

motivation for HF datasets? #127

Closed
Closed

Description

@arendu

The motivation behind these kind of "datasets" is very odd imo.

Why not just enforce a single dataset class - and call it a day! Anyone can write a simple script to download a dataset from HF and convert it to open-ai format, right? Also, there is almost zero usecase where you just take one dataset and train on it (at least to build high quality aligned models) its always a combination of a huge set of datasets, each with different formats etc. This is all data-munging work that a toolkit should not be entangled in.

Premature notions of "convenience" just end up being code debt.

Activity

  1. terrykong commented on Apr 3, 2025

    @terrykong
    Collaborator

    Allowing HF datasets in process is just to allow us to test things E2E and give users an easy way to get started.

    I am very anti-conversion scripts because they tend to be tech debt and have issues with maintenance and accumulate bugs due to being neglected.

    We are open to other input formats. Perhaps you can share your format and we can discuss an API.

    Premature notions of "convenience"

    We want the community to be able to reproduce our results, so that's something we also care about, so we can't just enforce a data schema and a conversion script and hope that the community can reproduce our results.

  2. titu1994 commented on Apr 3, 2025

    @titu1994

    I don't want dataset conversion scripts. HF + off the shelf jsonl support (using Datasets("json", ...)) similar to OpenRLHF is sufficient for most general tasks.

    If there are custom datasets, then writing a dataset class is anyway required (for multimodal for example)

  3. terrykong commented on Apr 3, 2025

    @terrykong
    Collaborator

    off the shelf jsonl support (using Datasets("json", ...))

    I think the schema for this is something we can discuss using this issue. I agree we should support a direct jsonl format for at least text datasets

  4. ashors1 commented on Apr 3, 2025

    @ashors1
    Contributor

    Even though our only existing examples are using HuggingFace datasets, that's definitely not all we plan to support. As described in the SFT documentation, the current design just expects your processed data to conform to the HuggingFace chat format to make it simple to use HuggingFace's tokenizer.apply_chat_template. How you massage your data into that format is up to you. You're totally free to work with arbitrary json files. @arendu let me know if you have concerns about this design. I'd also be happy to work with you on a specific use-case to showcase how to work with json files directly.

  5. arendu commented on Apr 3, 2025

    @arendu
    ContributorAuthor

    [The closer I look at it, I'm seeing this is can be somewhat minor/stylistic in nature, so my view is not as strong as it might seem from the first comment]
    [Also I'm only coming from the text-only dataset point of view]

    I guess Im getting mixed messages from the docs and the fact that data/hf_datasets/squad.py is a class and not a stand-alone toy script even though the line just before it says "If your data is not in the correct format, simply write a preprocessing script".

    To me, if it is a preprocessing script there should be no references to SquadDataset in the rest of the repo.

    Just something like ChatDataset is what I'd expect for a class reference in the rest of the repo. The e2e tests can certiainly have some logic to download HF and process it to fit into ChatDataset.

    that's definitely not all we plan to support.

    I'm actually advocating for actively supporting less :) I worry that it is not clear that SquadDataset is just for unit-tests and it looks like we are setting a precedence for "add your own dataset class for any new dataset you want to work with". (Which is not what the doc says, but the code looks like it saying that, at least to me).

  6. github-actions commented on Aug 3, 2025

    @github-actions

    This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

  7. locked and limited conversation to collaborators on Aug 4, 2025
  8. converted this issue into a discussion #830 on Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions