Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to handling of log directory #315

Merged
merged 3 commits into from
Jun 6, 2024
Merged

Changes to handling of log directory #315

merged 3 commits into from
Jun 6, 2024

Conversation

alexhernandezgarcia
Copy link
Owner

Summary of changes

  • Removal of quick fix about race conditions: before, a random directory was created for each run. Now, by default, the directory created by Hydra with the date and time in the name contains the microseconds, which would prevent the potential issues with race conditions.
  • A few default configurations have been changed:
    • TB loss instead of FM (because FM does not work for cont. environments)
    • Uniform proxy by default (because it works for all environments)
    • Hydra will not change directory by default.
  • I have remove "alex" as the default user in the experiment config files and removed "debug" from the logdir path.

Copy link
Collaborator

@michalkoziarski michalkoziarski left a comment

Choose a reason for hiding this comment

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

I believe this should be fine, but because you added SLURM_ID to the log dir; I am not totally sure if microseconds alone would be enough (no idea how precise Slurm is in starting jobs).

Added two discussion-type comments and one question that I think is important (on whether this works fine locally).

config/main.yaml Show resolved Hide resolved
config/main.yaml Show resolved Hide resolved
config/user/default.yaml Show resolved Hide resolved
@alexhernandezgarcia
Copy link
Owner Author

@michalkoziarski approved?

@alexhernandezgarcia alexhernandezgarcia merged commit 583eda5 into main Jun 6, 2024
1 check passed
@alexhernandezgarcia alexhernandezgarcia deleted the logdir branch June 6, 2024 18:26
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.

2 participants