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

Added a Hydra+wandb+submitit-launcher example #79

Merged
merged 24 commits into from
Jan 11, 2022

Conversation

zaccharieramzi
Copy link
Contributor

This solves #78

For now this wasnt tested, this is why I set it as a draft PR.

@mdiazmel
Copy link
Contributor

mdiazmel commented Jan 7, 2022

Cool! Thanks @zaccharieramzi

I would be happy to review this PR once ready. Don't hesitate to ping me.

@zaccharieramzi zaccharieramzi marked this pull request as ready for review January 7, 2022 14:25
@zaccharieramzi
Copy link
Contributor Author

Hi @mdiazmel ! Thanks for proposing to review. I think this can be reviewed now.

Copy link
Contributor

@mdiazmel mdiazmel left a comment

Choose a reason for hiding this comment

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

Great! Thanks @zaccharieramzi

I tested it in JZ and it works like a charm! From my side, I used the TF version installed from conda-forge and not the version packaged in the modulefile (as well the other dependencies). I added a couple of comments to clarify things in the README file.

It should be nice to include a direct link in the documentation web page (the vertical menu at the left). I can do that once the PR will be merged.

docs/examples/tf/tf_wandb_hydra/README.md Show resolved Hide resolved
docs/examples/tf/tf_wandb_hydra/README.md Show resolved Hide resolved
@zaccharieramzi
Copy link
Contributor Author

zaccharieramzi commented Jan 11, 2022

Hey @mdiazmel thanks for your review!
I have added a word on the sign in for wandb, as well as put the example in the index.

I can merge when you think it's ready.

@mdiazmel
Copy link
Contributor

LGTM.

I'll merge it!

@mdiazmel mdiazmel merged commit 912226b into jean-zay-users:master Jan 11, 2022
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.

3 participants