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

Add wandb-sweep example and clean-up use of configs #118

Merged
merged 23 commits into from
Feb 27, 2024
Merged

Conversation

julienroyd
Copy link
Contributor

@julienroyd julienroyd commented Feb 14, 2024

The use of both hps-dictionaries in the Task files was a bit difficult to manage alongside the Config() dataclass objects that were now becoming the norm inside the repo. The mix of both would:

  1. Make it difficult to keep-up in the task file with new configs and config name-changes in the Config objects since the dictionary structure needed to match the hierarchy in the config object without any direct dependency. IDE tools are much better at helping us track where a config is when using cfg.path.to.parameter than cfg{'path':{'to':{'parameter'}}}.
  2. Lead to silent errors since assuming a name change on the config side, the corresponding parameter would be defined both on the user side and in Config(), and these two parameter-twins would be used in different places in the code without any error getting triggered. By forcing the user to provide a Config() object to the constructor, we enforce the correct hierarchy more easily.

Also added support for wandb:

  • In the code but this is only optional and very light. In GFNTrainer.log() we simply check if a wandb.run is defined (whether wandb.init() has been called on the user side) and if so, we wandb.log() our metrics on the cloud too.
  • Added a wandb-sweep example in `gflownet/hyperopt/wandb-demo'. Just 2 files to run (1) set up hyperparameters and wandb sweep (2) launch slurm workers to instantiate wandb-agents and work on the sweep.

@julienroyd
Copy link
Contributor Author

@bengioe, it seemed harmless to me, but while cleaning up the config-use if the Tasks' main(), I moved qm9.py back with the other tasks. Not sure why historically it was the only task who had its own directory and config file but we should revert that if for some reason that was important.

@julienroyd julienroyd changed the title Remove hps dictionnaries in favour of Config() objects + minor clean-ups Add wandb-sweep example and clean-up use of configs Feb 16, 2024
@julienroyd julienroyd merged commit 96dde6b into trunk Feb 27, 2024
4 checks passed
@julienroyd julienroyd deleted the julien-wandb branch February 27, 2024 19:38
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