Skip to content

Conversation

@flferretti
Copy link
Contributor

This pull request introduces enhancements to the AMP loader by adding support for downloading datasets from Hugging Face using the login in https://github.com/ami-iit/amp-rsl-rl/blob/main/example/load_amp_dataset.py. The changes primarily focus on extending the AMPLoader class and its integration, along with minor updates to dependencies.

Enhancements to AMP functionality:

  • amp_rsl_rl/runners/amp_on_policy_runner.py: Updated the initialization of AMPLoader to include new parameters such as download_from_hf, robot_folder, and repo_id, enabling dataset downloads from Hugging Face.
  • amp_rsl_rl/utils/motion_loader.py: Modified the AMPLoader class to support optional parameters for downloading datasets from Hugging Face, including handling cache directories and repository details. Added a fallback to create a cache directory using platformdirs if dataset_path_root is not provided.

Dependency updates:

  • pyproject.toml: Added platformdirs>=4.0.0 to dependencies for managing cache directories required by the new Hugging Face dataset download functionality.

Minor improvements:

@flferretti flferretti force-pushed the feature/support_hf branch from 23ef898 to e0cecf2 Compare May 6, 2025 12:20
@Giulero
Copy link
Contributor

Giulero commented May 12, 2025

Thanks @flferretti! Had a look at it.
With this, is the dataset downloaded from ami hf, and just for ergoCub?
I'm a bit concerned about this, as ideally this library should also work for other robots!

@GiulioRomualdi
Copy link
Collaborator

GiulioRomualdi commented May 12, 2025

I would like to keep the motion_loader independent from Huggingface. There should be a function that allows the user to download the dataset from HF. I think it makes the usage of the repo a bit cleaner.

@GiulioRomualdi
Copy link
Collaborator

I would like to keep the motion_loader independent from Huggingface. There should be a function that allows the user to download the dataset from HF. I think it makes the usage of the repo a bit cleaner.

The reason why of this is also because there could be several different place where the dataset is stored.

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.

3 participants