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 artifact logging to wandb hook #1616

Merged
merged 11 commits into from
Jan 10, 2022
Merged

add artifact logging to wandb hook #1616

merged 11 commits into from
Jan 10, 2022

Conversation

fcakyon
Copy link
Contributor

@fcakyon fcakyon commented Dec 25, 2021

Problem: Currently it is not possible to upload experiment config files and model weights to wandb. However it very crucial to properly track experiments.

Solution: With this PR, users can upload the contents in work_dir to wandb after training ends utilizing log_artifact() method of wandb logger: https://docs.wandb.ai/ref/python/run#log_artifact

Usage:

log_config = dict(
    interval=1,
    hooks=[
        dict(
            type="WandbLoggerHook",
            init_kwargs=dict(
                entity=WANDB_ENTITY,
                project=WANDB_PROJECT,
                name=WANDB_NAME,
            ),
            log_artifact=True,
            out_suffix=('.log.json', '.log', '.py', '.pth')
        ),
    ],
)

@zhouzaida
Copy link
Collaborator

zhouzaida commented Dec 26, 2021

Hi @fcakyon, Should we provide an option to upload only the specified file suffix like mmcv/runner/hooks/logger/text.py , since some files are too large and not always needed.

https://docs.wandb.ai/ref/python/artifact#add_file

@fcakyon
Copy link
Contributor Author

fcakyon commented Dec 26, 2021

Hi @fcakyon, Should we provide an option to upload only the specified file suffix like mmcv/runner/hooks/logger/text.py , since some files are too large and not always needed.

https://docs.wandb.ai/ref/python/artifact#add_file

@zhouzaida its done!

@zhouzaida
Copy link
Collaborator

Hi @fcakyon, Should we provide an option to upload only the specified file suffix like mmcv/runner/hooks/logger/text.py , since some files are too large and not always needed.
https://docs.wandb.ai/ref/python/artifact#add_file

@zhouzaida its done!

👍

@zhouzaida
Copy link
Collaborator

It would be great if some unit tests are added.

fcakyon and others added 3 commits December 26, 2021 11:22
Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
@fcakyon
Copy link
Contributor Author

fcakyon commented Dec 26, 2021

It would be great if some unit tests are added.

its also done 👍

@zhouzaida
Copy link
Collaborator

It would be great if some unit tests are added.

its also done 👍

Wow, it is so great.

@fcakyon
Copy link
Contributor Author

fcakyon commented Dec 26, 2021

@zhouzaida it is ready 👍

Copy link
Contributor

@teamwong111 teamwong111 left a comment

Choose a reason for hiding this comment

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

It seems that the docstring lacks an args with_step.

@fcakyon
Copy link
Contributor Author

fcakyon commented Dec 27, 2021

@teamwong111 i have no idea what with_step parameter does, i did not add that parameter, it was there before.

@zhouzaida
Copy link
Collaborator

@teamwong111 i have no idea what with_step parameter does, i did not add that parameter, it was there before.

The details about the argument can be found at #913

@fcakyon
Copy link
Contributor Author

fcakyon commented Dec 27, 2021

@zhouzaida it is ready to be merged now 👍

Copy link
Contributor

@teamwong111 teamwong111 left a comment

Choose a reason for hiding this comment

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

Nice, thank you for your contribution

@fcakyon
Copy link
Contributor Author

fcakyon commented Jan 5, 2022

@zhouzaida @teamwong111 when will this be merged? i need it urgently :)

@zhouzaida
Copy link
Collaborator

@zhouzaida @teamwong111 when will this be merged? i need it urgently :)

Hi @fcakyon , we will merge the PR in the next few days.

@ZwwWayne ZwwWayne merged commit 4051832 into open-mmlab:master Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants