Add logger to Weights & Biases #607

borisdayma · 2020-03-10T00:12:21Z

Add support for logging through Weights & Biases.

WandbLogger automatically logs metrics, model topology, gradients and best trained model.

Sample run: https://app.wandb.ai/borisd13/skorch/runs/1vs6725x?workspace=user-borisd13
You can switch tabs on the left and see computer resources, model graph, logged files, etc

borisdayma · 2020-03-10T00:12:50Z

I'm now working on adding tests. Feel free if you have any comments.

BenjaminBossan · 2020-03-11T21:28:56Z

Thanks for this PR.

You've probably seen the Tensorboard and NeptuneLogger callbacks. There we adopt a pattern of passing the actual logger instance to the callback (e.g. the SummaryWriter) instead of using the try: import ... pattern inside the callback. Would it be possible to change WandbLogger in a similar fashion?

borisdayma · 2020-03-12T05:59:32Z

Yes of course, I'll make the changes.

…at-wandb

BenjaminBossan

Thank you very much for adding this feature to skorch and implementing my suggestion. There are a few things that need to be changed before we can merge this. Some of them I have commented on. Apart from that:

Please implement unit tests for your new callback. You could have a look at how it's done for the NeptuneLogger.
Please add W&B to the requirements-dev.txt.
For me it's not quite clear how I can test this in practice. I could probably figure this out by digging through the docs of W&B but I think it would be much nicer for me -- and future users -- if the callback's documentation explained everything that is required. Do I need to start a local server? Or can I even use a test server that you provide? Do I need to sign up or is there a way to use this without?

BenjaminBossan · 2020-03-14T21:56:20Z

skorch/callbacks/logging.py

+class WandbLogger(Callback):
+    """Logs best model and metrics to `Weights & Biases <https://docs.wandb.com/>`_
+
+    "Use this callback to automatically log best trained model and all metrics from


Suggested change

"Use this callback to automatically log best trained model and all metrics from

Use this callback to automatically log best trained model and all metrics from

BenjaminBossan · 2020-03-14T22:00:21Z

skorch/callbacks/logging.py

+    """Logs best model and metrics to `Weights & Biases <https://docs.wandb.com/>`_
+
+    "Use this callback to automatically log best trained model and all metrics from
+    your net's history to Weights & Biases after each epoch.


I think you should use the docstring to help skorch users who are unfamiliar with W&B to get started quickly. E.g., you could specify what package they must install for this to work, i.e. a pip (or conda) instruction. You should also indicate what kind of setup they need to make beforehand (say, starting a local server).

For a nice example, look at the docstring for NeptuneLogger.

BenjaminBossan · 2020-03-14T22:00:53Z

skorch/callbacks/logging.py

+    >>> import wandb
+    >>> from skorch.callbacks import WandbLogger
+    >>> wandb_run = wandb.init()
+    >>> wandb.config.update({"learning rate": 1e-3, "batch size": 32})  # optional


Could you indicate what this config update does?

BenjaminBossan · 2020-03-14T22:02:08Z

skorch/callbacks/logging.py

+    """
+
+    # Record if watch has been called previously (even in another instance)
+    _watch_called = False


Is this really used anywhere? If it is, please move this inside initialize and call it watch_called_.

BenjaminBossan · 2020-03-14T22:03:26Z

skorch/callbacks/logging.py

+        self.wandb_run = wandb_run
+        self.save_model = save_model
+        self.keys_ignored = keys_ignored
+        self.model_path = Path(wandb_run.dir) / 'best_model.pth'


Please don't set any arguments in __init__ that are not passed by the user. So either allow them to pass the model_path argument (if that makes any sense) or instead set the model_path inside initialize (and call it model_path_).

BenjaminBossan · 2020-03-14T22:04:40Z

skorch/callbacks/logging.py

+        """Automatically log values from the last history step."""
+        hist = net.history[-1]
+        keys_kept = filter_log_keys(hist, keys_ignored=self.keys_ignored_)
+        logged_vals = dict((k, hist[k]) for k in keys_kept if k in hist)


Suggested change

logged_vals = dict((k, hist[k]) for k in keys_kept if k in hist)

logged_vals = {k: hist[k] for k in keys_kept if k in hist}

BenjaminBossan · 2020-03-14T22:05:34Z

skorch/callbacks/logging.py

+      wandb Run used to log data.
+
+    save_model : bool (default=True)
+      Saves best trained model.


Suggested change

Saves best trained model.

Whether to save a checkpoint of the best model.

BenjaminBossan · 2020-03-14T22:06:29Z

skorch/callbacks/logging.py

+    def __init__(
+            self,
+            wandb_run,
+            save_model=True,


We already provide a checkpoint callback, I think this functionality is redundant.

This is to log the trained model to W&B.

Interesting. How does that work? In this code, I don't see any interaction with W&B:

# save best model if self.save_model and hist['valid_loss_best']: model_path = Path(self.wandb_run.dir) / 'best_model.pth' with model_path.open('wb') as model_file: net.save_params(f_params=model_file)

Is this some code working in the background or is it simply the fact that the model parameters are stored in the wandb_run_dir?

All files stored in wandb_run.dir are automatically saved.
You can see in my example run on the "files" tab

Please leave a comment that states that the files in wandb_run.dir is automatically saved in on_epoch_end.

borisdayma · 2020-03-15T22:50:22Z

Thanks for the comments @BenjaminBossan
They should now be addressed but feel free to add anything.

I am using a new feature (wandb.run.watch) that has not been pushed to pypi yet so I'll update the requirements-dev with the correct version as soon as it's pushed.

BenjaminBossan · 2020-03-16T23:52:58Z

@borisdayma Thank you for implementing the suggested changes. I will review this in detail when I have more time, hopefully soon.

borisdayma · 2020-03-17T05:21:26Z

I just need to pin wandb version as soon as next release appears

borisdayma · 2020-03-20T00:22:49Z

It should now be ready to be merged! Feel free if you have any questions or comments.

BenjaminBossan

This looks good to me. I tested it and the logs are really useful. Thanks for the addition.

I have a minor comment about how to update the docstring. On top of that, if there is the option to run W&B without an account (i.e. locally), it would be nice to mention that. I believe this will reduce the friction for users who want to test this.

@ottonemo @thomasjpfan do you want to take a look at this as well? Otherwise I think we can merge as soon as the docs have been updated.

BenjaminBossan · 2020-03-22T14:30:17Z

skorch/callbacks/logging.py

+      wandb Run used to log data.
+
+    save_model : bool (default=True)
+      Whether to save a checkpoint of the best model.


Maybe add an explanation here that the model will be uploaded to the W&B server for convenience.

borisdayma · 2020-03-22T16:37:50Z

@BenjaminBossan Glad you found it useful!

I updated the docstring and added a comment for logging anonymously (without a W&B account). Feel free if you want me to change the wording or anything else.

thomasjpfan

Thank you for the PR @borisdayma

thomasjpfan · 2020-03-23T00:15:36Z

skorch/callbacks/logging.py

+    ... wandb_run = wandb.init(anonymous="allow)
+
+    >>> # Log hyper-parameters (optional)
+    ... wandb.config.update({"learning rate": 1e-3, "batch size": 32})


Is there a way to update this using the wandb_run object?

Yes! I'll update

thomasjpfan · 2020-03-23T00:17:50Z

skorch/callbacks/logging.py

+    def __init__(
+            self,
+            wandb_run,
+            save_model=True,


Please leave a comment that states that the files in wandb_run.dir is automatically saved in on_epoch_end.

thomasjpfan · 2020-03-23T00:20:17Z

skorch/tests/callbacks/test_logging.py

+        return mock
+
+    @pytest.fixture
+    def net_fitted(


This fixture is never used and can be removed.

skorch/callbacks/logging.py

Co-Authored-By: Thomas J Fan <[email protected]>

borisdayma · 2020-03-23T02:14:15Z

All comments should have been implemented in last commit. Let me know if I missed anything.

borisdayma · 2020-03-30T16:37:18Z

Just checking if you want me to add anything else to this PR

BenjaminBossan · 2020-03-31T19:14:01Z

@borisdayma From my side, it looks good.

I had requested a re-review from @thomasjpfan since you addressed his comments. Maybe let's give him until the weekend to respond, if he doesn't we can consider it a thumbs up and merge.

thomasjpfan

LGTM Thank you @borisdayma !

BenjaminBossan · 2020-04-05T18:12:35Z

Good job everyone, @borisdayma thanks for the contribution and your patience.

borisdayma · 2020-04-05T19:32:39Z

Thanks, I think it will be very useful for fine-tuning. Feel free to reach if there's any issue related to it.

BenjaminBossan · 2020-04-11T08:26:32Z

@borisdayma I now notice that conda install -c conda-forge --file requirements-dev.txt fails with the following error:

PackagesNotFoundError: The following packages are not available from current channels:
  - wandb[version='>=0.8.30']

Is there a conda channel for wandb or is pip the only solution?

borisdayma · 2020-04-11T16:53:41Z

At the moment it is only on pip but we can just use the new conda interoperability with pip

borisdayma · 2020-04-11T18:23:08Z

Let me know if you want me to try it and suggest a new PR (to update docs).
I personally use pipenv all the time so I had not noticed this issue with conda.

borisdayma added 2 commits March 9, 2020 19:05

feat(wandb): add logger to Weights & Biases

79ead54

docs(changes.md): add reference to WandbLogger

10fdefa

borisdayma added 2 commits March 12, 2020 20:05

feat(wandb): add run instance to callback

d17c35a

Merge branch 'master' of https://github.com/skorch-dev/skorch into fe…

3864919

…at-wandb

BenjaminBossan requested changes Mar 14, 2020

View reviewed changes

borisdayma added 5 commits March 15, 2020 13:34

test(wandb): added tests

5d8f780

feat(requirements-dev.txt): add wandb

5907832

docs(wandb): add documentation

5ef1bde

feat(wandb): update doc

e54fe59

refactor(wandb): address comments

2991565

feat(wandb): remove ref to _watch_called

9198011

feat(wandb): set minimum version

3ca5741

BenjaminBossan approved these changes Mar 22, 2020

View reviewed changes

docs(wandb): log anonymous + upload model

5785ac0

thomasjpfan reviewed Mar 23, 2020

View reviewed changes

borisdayma and others added 2 commits March 22, 2020 21:08

feat(wandb): simplify logged_vals

7d3a775

Co-Authored-By: Thomas J Fan <[email protected]>

feat(wandb): implement comments

878b619

BenjaminBossan requested a review from thomasjpfan March 25, 2020 19:08

thomasjpfan approved these changes Apr 5, 2020

View reviewed changes

BenjaminBossan merged commit e61f10c into skorch-dev:master Apr 5, 2020

BenjaminBossan mentioned this pull request Apr 13, 2020

[WIP] Update README and enviroment #616

Merged

	"Use this callback to automatically log best trained model and all metrics from
	Use this callback to automatically log best trained model and all metrics from

	logged_vals = dict((k, hist[k]) for k in keys_kept if k in hist)
	logged_vals = {k: hist[k] for k in keys_kept if k in hist}

	Saves best trained model.
	Whether to save a checkpoint of the best model.

Add logger to Weights & Biases #607

Add logger to Weights & Biases #607

Conversation

borisdayma commented Mar 10, 2020

borisdayma commented Mar 10, 2020

BenjaminBossan commented Mar 11, 2020

borisdayma commented Mar 12, 2020

BenjaminBossan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borisdayma commented Mar 15, 2020

BenjaminBossan commented Mar 16, 2020

borisdayma commented Mar 17, 2020 • edited Loading

borisdayma commented Mar 20, 2020

BenjaminBossan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borisdayma commented Mar 22, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borisdayma commented Mar 23, 2020

borisdayma commented Mar 30, 2020

BenjaminBossan commented Mar 31, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

BenjaminBossan commented Apr 5, 2020

borisdayma commented Apr 5, 2020

BenjaminBossan commented Apr 11, 2020

borisdayma commented Apr 11, 2020

borisdayma commented Apr 11, 2020

BenjaminBossan left a comment •

edited

Loading

borisdayma commented Mar 17, 2020 •

edited

Loading