fix(wandb): prevent WandbLogger from dropping values

[borisdayma]

What does this PR do?

This PR simplifies use of WandbLogger.

PR #5351 introduced a workaround for dealing with non monotonous steps.
The consequence was that some logged values would get dropped. It was suggested to the user to use sync_step=False, forcing the user to rerun his experiment.

The new behavior:

• the step is logged as train/step
• if using latest wandb version, it will automatically be set as x-axis by default
• no need to use wandb.log(items, commit=False) to prevent step from auto-increasing

This will prevent any value from being dropped.

Should step not be increasing for any reason, users will be able to select a different x-axis.

There are no breaking changes because the current version (with sync_step) had not been released yet.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified
• Check that target branch and milestone match!

Did you have fun?

Make sure you had fun coding 🙃

[pep8speaks]

Hello @borisdayma! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-02-27 01:26:56 UTC

[codecov]

Codecov Report

Merging #5931 (160c16b) into master (ee5032a) will decrease coverage by 2%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #5931    +/-   ##
=======================================
- Coverage      93%     91%    -2%     
=======================================
  Files         159     159            
  Lines       11384   11380     -4     
=======================================
- Hits        10628   10358   -270     
- Misses        756    1022   +266     

[borisdayma]

It's now ready for review.
We won't see any changes in the UI as of now (ie default x-axis won't be auto-set) but it will automatically appear with the next release of wandb.

It would be great to have it merged before people have a chance to use the "sync_step" functionality (which is in master but not released yet).

Let me know if you have any comments. I'll fix the simple merge issues.

[Borda]

@borisdayma mind resolve conflicts?

[anmol-wiai]

Hi,

Note: When logging manually through `wandb.log` or `trainer.logger.experiment.log`,
make sure to use `commit=False` so the logging step does not increase.

Is this note relevant after this PR?