Skip to content

Code review notes #79

@RedTachyon

Description

@RedTachyon

Writing this without fixes for now, things I'm noticing going through the library:

  1. The dataset listed in README doesn't seem to exist. This is bad, as that's the first thing a beginner will likely try when trying to interact with the library. We should either swap it, or create it.
  2. Remote datasets are 'door-cloned-v0', 'door-expert-v0', 'door-human-v0', ..., at least reading it initially. One issue here is that the dataset names don't really tell you much unless you already know the context - we should probably mention that they're specifically for mujoco, probably specifically for some specific model - maybe we can list the name of the gymnasium env?
  3. Trying to do minari.download_dataset("door-human-v0") results in a pretty ugly/uncaught import error for gymnasium_robotics. Since gymnasium-robotics isn't a necessary dependency, we should have a more elegant way of handling this.
  4. (3a) The import issue seems to happen because when downloading the dataset, we're creating the environment to get the obs and action spaces. Ignoring even the dependency issue, this seems very redundant - can't we just save the obs/action spaces in the dataset? They should be easy to serialize.
  5. Downloading the same dataset twice repeats the download and overwrites it. We explicitly acknowledge this, but imo there should be an overwrite or force argument to do that behavior, and by default it should just skip downloading it. It might be worthwhile to think if we want to do explicit downloads like this in the first place, but that's another thing.
  6. EpisodeData is a namedtuple, which leads to the default representation being an enormous printout of all observations, rewards etc. This is not very useful, so we should probably make it a proper dataclass instead, and override the default representation.

Stopping here for now, @rodrigodelazcano let me know if there's anything I'm missing anything for any of those points (like some additional reasons for doing X thing in a certain way that I'm unaware of). Otherwise we can make this into a to-do list of improvements

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions