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

Decouple environment creation and configuration file parsing #930

Closed
4 tasks
breakds opened this issue Jul 15, 2021 · 3 comments
Closed
4 tasks

Decouple environment creation and configuration file parsing #930

breakds opened this issue Jul 15, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@breakds
Copy link
Contributor

breakds commented Jul 15, 2021

Motivation

In the current implementation, environment processes are created at the moment when the configuration is being parsed/loaded. I would propose to decouple those two because:

  1. In the situation when we want to start multiple trainer to run training in parallel, it is preferred that configuration is parsed and loaded first, and each trainer process will start the environments by themselves. We need an option to separate configuration parsing and environment process creation.
  2. For readability, it is more clear if parse_conf_file does not create the environment under the hood. In fact, explicitly creating them seems more clear.

Goal

  • Separate the concept of configuration parsing and environment creation in the current implementation.
  • Such change should be transparent to most of the alf developers
  • Update the related documentation
  • Test and verify the change
@breakds breakds added the enhancement New feature or request label Jul 15, 2021
@breakds breakds self-assigned this Jul 15, 2021
@hnyu
Copy link
Collaborator

hnyu commented Jul 15, 2021

I think there is a very complicated reason why creating env is inside parse_conf_file. I suggest asking @emailweixu first before making any change. It should be related to how alf config works and env random seed initialization.

@breakds
Copy link
Contributor Author

breakds commented Jul 15, 2021

Thanks for the reminder. Yep I understand that this is related to the seed. Will confirm with @emailweixu.

@breakds
Copy link
Contributor Author

breakds commented Aug 24, 2021

Closing this because it depends on retiring the .gin files completely (or to put it in another way, waiting for the retirement of .gin is an easier alternative). Will re-open once that is done.

@breakds breakds closed this as completed Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants