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

Change the command to initialize DDP subprocess with Hydra #16694

Closed
zihaozou opened this issue Feb 8, 2023 · 2 comments · Fixed by #18145
Closed

Change the command to initialize DDP subprocess with Hydra #16694

zihaozou opened this issue Feb 8, 2023 · 2 comments · Fixed by #18145
Labels
3rd party Related to a 3rd-party feature Is an improvement or enhancement strategy: ddp DistributedDataParallel won't fix This will not be worked on

Comments

@zihaozou
Copy link

zihaozou commented Feb 8, 2023

Description & Motivation

Hi,

the current behavior to initialize DDP subprocesses with Hydra is here, which overrides the hydra.run.dir with os.getcwd().

This is assuming the os.getcwd() will return the output directory that Hydra assigned. However, since Hydra 1.2, Hydra will keep the cwd as the project directory by default. This command will result in logging the subprocess in the project directory.

Pitch

Logging the DDP subprocess correctly in the output directory created by Hydra

Alternatives

use:

os_cwd = hydra.core.hydra_config.HydraConfig.get()['runtime']['output_dir']
os_cwd = f'"{os_cwd}"'

instead

Additional context

No response

cc @Borda @justusschock @awaelchli

@zihaozou zihaozou added feature Is an improvement or enhancement needs triage Waiting to be triaged by maintainers labels Feb 8, 2023
@awaelchli
Copy link
Member

@zihaozou Thanks for the request.

I remember we had changed the behavior once around this (#11617) but had to revert it (#15737).
It looks like this is about the same thing, but I'm not sure.

@awaelchli awaelchli added 3rd party Related to a 3rd-party strategy: ddp DistributedDataParallel and removed needs triage Waiting to be triaged by maintainers labels Feb 9, 2023
@stale
Copy link

stale bot commented Mar 19, 2023

This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions - the Lightning Team!

@stale stale bot added the won't fix This will not be worked on label Mar 19, 2023
@stale stale bot closed this as completed Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party Related to a 3rd-party feature Is an improvement or enhancement strategy: ddp DistributedDataParallel won't fix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants