Skip to content

Infrastructure for logging to wandb - #758

Merged
tamarl08 merged 4 commits into
mainfrom
wandb_infra
Dec 10, 2023
Merged

Infrastructure for logging to wandb#758
tamarl08 merged 4 commits into
mainfrom
wandb_infra

Conversation

@tamarl08

@tamarl08 tamarl08 commented Dec 6, 2023

Copy link
Copy Markdown
Contributor

This PR adds initial infrastructure for W&B logging, including a configuration field to enable W&B, a wand interface class, documentation and tests.

@pep8speaks

pep8speaks commented Dec 6, 2023

Copy link
Copy Markdown

Hello @tamarl08! Thanks for updating this PR.

Line 1852:101: E501 line too long (105 > 100 characters)
Line 1859:101: E501 line too long (104 > 100 characters)

Comment last updated at 2023-12-08 16:02:50 UTC

@codecov

codecov Bot commented Dec 6, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8ac783c) 95.30% compared to head (78541a1) 95.33%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #758      +/-   ##
==========================================
+ Coverage   95.30%   95.33%   +0.02%     
==========================================
  Files          29       30       +1     
  Lines        3580     3598      +18     
==========================================
+ Hits         3412     3430      +18     
  Misses        168      168              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@desilinguist desilinguist left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a reason the coverage checks are not being met?

Comment thread doc/run_experiment.rst Outdated
Comment thread skll/config/__init__.py Outdated
@tamarl08

tamarl08 commented Dec 8, 2023

Copy link
Copy Markdown
Contributor Author

Is there a reason the coverage checks are not being met?

I added a test for run_configuration with W&B and now it's covered. I don't know why it did not count as covered before, those lines are tested at test_wandb.py

@desilinguist desilinguist left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great! Thanks for adding the new test!

@tamarl08
tamarl08 merged commit 7c09d07 into main Dec 10, 2023
@delete-merged-branch
delete-merged-branch Bot deleted the wandb_infra branch December 10, 2023 19:24
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.

4 participants