Skip to content

Conversation

@ozhanozen
Copy link
Contributor

Description

Due to previous changes, the rl_games workflow's log_root_path is no longer the absolute path if the pbt option is not used, causing further issues. This PR fixes this by making it an absolute path again.

Fixes #3530

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Before:

log_root_path = os.path.join("logs", "rl_games", config_name)
if "pbt" in agent_cfg:
if agent_cfg["pbt"]["directory"] == ".":
log_root_path = os.path.abspath(log_root_path)
else:
log_root_path = os.path.join(agent_cfg["pbt"]["directory"], log_root_path)

After:

log_root_path = os.path.join("logs", "rl_games", config_name)
if "pbt" in agent_cfg and agent_cfg["pbt"]["directory"] != ".":
    log_root_path = os.path.join(agent_cfg["pbt"]["directory"], log_root_path)
else:
    log_root_path = os.path.abspath(log_root_path)

Note

While this fixes the path to be absolute when pbt is not used, I am not sure if

log_root_path = os.path.join(agent_cfg["pbt"]["directory"], log_root_path)

is correct or absolute, as I do not use pbt. Should it not be something like the following?

log_root_path = os.path.abspath(os.path.join(log_root_path, agent_cfg["pbt"]["directory"]))

I would appreciate any feedback on this.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Copy link
Collaborator

@garylvov garylvov left a comment

Choose a reason for hiding this comment

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

LGTM!

@ozhanozen
Copy link
Contributor Author

Hi @ooctipus, would love to get your opinion before merging on the line:
log_root_path = os.path.join(agent_cfg["pbt"]["directory"], log_root_path)
so we are sure that the pbt log directory is also correct.

@ooctipus
Copy link
Collaborator

sorry for the late response, yes I believe this is correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug Report] rl_games workflow's log_root_path is not the absolute path when pbt is not used

4 participants