Skip to content

[WB-6947] Flush log data without finish with service#3137

Merged
raubitsj merged 37 commits intomasterfrom
WB-6947-1
Feb 22, 2022
Merged

[WB-6947] Flush log data without finish with service#3137
raubitsj merged 37 commits intomasterfrom
WB-6947-1

Conversation

@kptkin
Copy link
Copy Markdown
Collaborator

@kptkin kptkin commented Jan 12, 2022

Fixes WB-6947

Description

What does the PR do?

This PR moves the partial history logic to the internal process. This PR allows for runs that use the wandb-service path to flush their remaining history without explicitly calling wandb.finish.

This is the first PR in series of PR that move the step tracking logic from the user process to the internal process.

Testing

How was this PR tested?
This PR should keep functionality, it is tested using the existing tests.

Checklist

  • Name PR "[WB-NNNN][WB-MMMM] Add support for..." similar to entries in CHANGELOG.md
  • Include reference to internal ticket "Fixes WB-NNNN" (and github issue "Fixes #NNNN" if applicable)

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 12, 2022

Codecov Report

Merging #3137 (ed53556) into master (0976290) will increase coverage by 0.06%.
The diff coverage is 96.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3137      +/-   ##
==========================================
+ Coverage   79.90%   79.96%   +0.06%     
==========================================
  Files         213      212       -1     
  Lines       27882    27908      +26     
==========================================
+ Hits        22278    22316      +38     
+ Misses       5604     5592      -12     
Flag Coverage Δ
functest 56.86% <81.77%> (+0.06%) ⬆️
unittest 69.35% <85.71%> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wandb/sdk/__init__.py 100.00% <ø> (ø)
wandb/sdk/wandb_watch.py 74.54% <80.00%> (-0.90%) ⬇️
wandb/wandb_torch.py 59.93% <83.33%> (+0.06%) ⬆️
wandb/sdk/data_types.py 84.31% <85.71%> (-0.04%) ⬇️
wandb/integration/tensorboard/log.py 86.79% <93.75%> (+0.12%) ⬆️
wandb/sdk/interface/interface.py 89.62% <94.44%> (+0.22%) ⬆️
wandb/sdk/internal/handler.py 90.61% <98.48%> (+0.25%) ⬆️
wandb/data_types.py 83.68% <100.00%> (+0.01%) ⬆️
wandb/integration/tensorboard/__init__.py 100.00% <100.00%> (ø)
wandb/integration/tensorflow/estimator_hook.py 83.87% <100.00%> (+1.72%) ⬆️
... and 16 more

Comment thread wandb/sdk/wandb_run.py Outdated
Comment thread wandb/wandb_torch.py Outdated
@kptkin kptkin requested a review from raubitsj January 18, 2022 20:21
Comment thread wandb/integration/tensorboard/log.py Outdated
Comment thread wandb/sdk/internal/handler.py Outdated
Comment thread wandb/sdk/wandb_run.py Outdated
@kptkin kptkin marked this pull request as ready for review February 17, 2022 05:05
@raubitsj raubitsj added this to the sdk-2022-03.1 milestone Feb 18, 2022
@kptkin kptkin requested a review from KyleGoyette February 18, 2022 18:50
Copy link
Copy Markdown
Contributor

@raubitsj raubitsj left a comment

Choose a reason for hiding this comment

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

PERFECT!

Comment thread wandb/wandb_torch.py
if name is not None:
prefix = prefix + name

if jupyter_run:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Im just curious. did you look into why this was here and what it was accomplishing before?

Copy link
Copy Markdown
Collaborator Author

@kptkin kptkin Feb 22, 2022

Choose a reason for hiding this comment

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

I couldn't find what is was doing... I saw it is used to recover history...
I guess I can keep it and just use it as a run... (if provided)

I did a simple example in a Notebook without it and it worked, but it's not a guarantee

Comment thread wandb/sdk/wandb_watch.py
log_gradients=log_gradients,
prefix=prefix,
log_freq=log_freq,
jupyter_run=wandb.run if in_jupyter else None,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it is using the same wandb.run so that's why i though it's ok to remove

@raubitsj raubitsj merged commit 924d0bc into master Feb 22, 2022
@raubitsj raubitsj deleted the WB-6947-1 branch February 22, 2022 21:43
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