Added Neptune logging #586

jakubczakon · 2020-02-03T12:55:01Z

Added Neptune logger that:

logs metrics on_batch_end
logs metrics on_epoch_end
logs any additional information directly to neptune_logger.experiment.log_whatever_neptune_allows if used with close_after_train=False

BenjaminBossan · 2020-02-03T21:27:02Z

@jakubczakon Thanks for the addition, I will have a thorough look at it soon.

What would be the easiest way for me to test this? Is the example in the docstring working or do I need to create a neptune account?

jakubczakon · 2020-02-03T21:44:20Z

That's a fair point @BenjaminBossan

The example in the docstrings (link to experiment) works.
You can copy and run the code but you would need to create a (free) account.

I think a decent solution to that would be to use the public token (anonymous mode) not to force anyone to register.
What do you think about that?

BenjaminBossan · 2020-02-03T21:56:50Z

I think a decent solution to that would be to use the public token (anonymous mode) not to force anyone to register.
What do you think about that?

Yes, I think everything that reduces the friction to try it out would be helpful.

jakubczakon · 2020-02-04T10:39:12Z

Done, now anyone can run the example without registering and see their experiments in neptune.

BenjaminBossan · 2020-02-04T20:27:37Z

Nice, I will give this a spin soon and come back with detailed feedback. From a first glance, it looks very good though.

BenjaminBossan

Excellent addition, thank you very much Jakub. Nice to see how little is actually needed to make the integration work.

There are a few minor comments, please have a look at them. I think the biggest question that came up for me was if we really need batch level logging. At least for the given example, it's very noisy, maybe epoch level is enough?

Also, please add a sentence to the CHANGES.md.

BenjaminBossan · 2020-02-06T19:56:52Z

skorch/callbacks/logging.py

+    your net's history to Neptune.
+
+    The best way to log additional information is to log directly to the
+    experiment object or subclass the `on_*`` methods.


Suggested change

experiment object or subclass the `on_*`` methods.

experiment object or subclass the ``on_*`` methods.

BenjaminBossan · 2020-02-06T20:00:36Z

skorch/callbacks/logging.py

+    keys_ignored : str or list of str (default=None)
+      Key or list of keys that should not be logged to
+      Neptune. Note that in addition to the keys provided by the
+      user.


Suggested change

user.

user, keys such as those starting with 'event_' or ending on

'_best' are ignored by default.

BenjaminBossan · 2020-02-06T20:01:56Z

skorch/callbacks/logging.py

+    Note
+    ----
+
+    Install psutil to monitor resource consumption


Kinda redundant with the comment on line 76.

BenjaminBossan · 2020-02-06T20:13:42Z

skorch/callbacks/logging.py

+    >>> # Create a neptune experiment object
+    ... # We are using api token for an anonymous user.
+    ... # For your projects use the token associated with your neptune.ai account
+    >>> neptune.init(api_token='eyJhcGlfYWRkcmVzcyI6Imh0dHBzOi8vdWkubmVwdHVuZS5tbCIsImFwaV9rZXkiOiJiNzA2YmM4Zi03NmY5LTRjMmUtOTM5ZC00YmEwMzZmOTMyZTQifQ==',


Maybe you could add the install instruction of neptune, as well as import neptune to the code example.

BenjaminBossan · 2020-02-06T20:21:33Z

skorch/callbacks/logging.py

+        self.keys_ignored_.add('batches')
+        return self
+
+    def on_batch_end(self, net, **kwargs):


I wonder if we really need batch level logging. Maybe logging at epoch level is sufficient? At least, I think it would make sense to allow to turn off batch level logging through a parameter.

I often find having batch-level logging valuable but I agree there should be an option to turn it off.
Added it.

BenjaminBossan · 2020-02-06T20:21:55Z

skorch/tests/callbacks/test_logging.py

+    def test_fit_with_dict_input(
+            self,
+            net_cls,
+            classifier_module,


argument not used (probably copied from tensorboard test that also doesn't need it). Tbh, I think this whole test can be removed here.

…efault

jakubczakon · 2020-02-07T09:08:05Z

I see that I messed up the formatting in some places with my local settings.
That sais, it passes pylint (with warnings)
Should I fix those, or they are not important to you?

BenjaminBossan

I think the optional batch-level logging is a good compromise. Would you be so kind to add a test for that? At the moment, this functionality is uncovered. You could, e.g., pass a mock object as Experiment and count how often it was called or what it was called with (maybe reduce the batch size to trigger multiple batch-level calls).

Regarding the formatting changes: Some of them are good, some unnecessary but not harmful. I don't care either way.

jakubczakon · 2020-02-10T12:10:01Z

I've added this test @BenjaminBossan,
Interestingly this test passes locally, calling my `.log_metric 130 times, yet on github it calls it only 120 times.
I will investigate but seems unexpected.

BenjaminBossan · 2020-02-10T20:07:13Z

@jakubczakon I tried to understand the numbers. For this, I turned off the internal validation, as it makes the whole calculation more difficult (by adding train_split=False as argument to the net).

Now we have the following situation:

on batch level, we have 2 keys x (40 / 4) batches x 5 epochs = 100 calls
on epoch level, we have 2 keys x 5 epochs = 10 calls

That is 110 in sum, which is exactly what I find (please add this explanation to the comment for future reference).

I think it would also make sense to copy the same test, but this time with log_on_batch_end=False. Then we should expect 10 calls.

Why you get different results locally, I don't know (for me, it's the same as in the CI). My first guess would be some kind of version mismatch.

…nts, added test for when batch log is off

jakubczakon · 2020-02-11T08:38:49Z

Thank you @BenjaminBossan!

I've added the suggestions and it fixed the issue.

BenjaminBossan · 2020-02-11T20:10:47Z

Thanks @jakubczakon, this looks very good to me now.

I will wait a few days to see if @ottonemo or @thomasjpfan have any comments on this, otherwise I will merge it.

jakubczakon · 2020-02-12T07:10:40Z

Awesome, thanks!

thomasjpfan · 2020-02-13T16:05:42Z

skorch/callbacks/logging.py

+        self.keys_ignored = keys_ignored
+
+    def initialize(self):
+        self.first_batch_ = True


Is first_batch_ used?

I think this is for consistency with the TensorBoard callback. It is convenient to have so that you can, e.g., log an image of the network graph exactly once. You may not be able to use on_train_begin for this because that one gets the input X, not the one that is returned by the data loader.

Yeah, I simply copied it from TensorBoard (to be honest I haven't thought about it much).

Also, if I were to use it properly I should have self.first_batch_ = False on on_batch_end which is missing.

I've added self.first_batch_ = False to on_batch_end but I can easily drop it from both as they are not used (to my understanding)

What do you think?

The main reason I wanted to have it for TensorBoard was to be able to trace and add a graph of the network to TensorBoard. I think that option doesn't exist for neptune, does it? However, I think consistency is also nice, so I would leave it there.

Let's document the attribute in the docstring and add a quick test for first_batch_?

Yes, good idea.

thomasjpfan · 2020-02-13T16:08:15Z

skorch/tests/callbacks/test_logging.py

+            classifier_module,
+            callbacks=[npt],
+            max_epochs=3,
+        ).fit(*data)


Can we assert how many times log_metric should be called in this case?

Sure can do.

…eys are ignored

thomasjpfan · 2020-02-14T21:03:41Z

skorch/callbacks/logging.py

+        self.keys_ignored = keys_ignored
+
+    def initialize(self):
+        self.first_batch_ = True


Let's document the attribute in the docstring and add a quick test for first_batch_?

jakubczakon · 2020-02-15T22:20:52Z

Done @thomasjpfan

jakubczakon · 2020-02-16T08:18:34Z

Awesome!
Thank you @BenjaminBossan @thomasjpfan !

BenjaminBossan

Thanks for the great work and taking the time to address the comments.

jakubczakon added 3 commits February 2, 2020 14:16

initial neptune logging added

e4e3c0e

added neptune logging tests

7387aed

updated link

114284e

jakubczakon requested a review from BenjaminBossan February 3, 2020 12:55

jakubczakon closed this Feb 3, 2020

added link to an example experiment ru

91602af

jakubczakon reopened this Feb 3, 2020

fixed formatting

af9d165

changed api token to the one for anonymous user

58df275

BenjaminBossan requested changes Feb 6, 2020

View reviewed changes

jakubczakon added 2 commits February 7, 2020 09:41

local pre-merge

efc2af0

fixed minor format problems, added log_on_batch_end flag (False) by d…

38a266f

…efault

BenjaminBossan reviewed Feb 8, 2020

View reviewed changes

added batch-level test

dc2e8e6

switched to train_split=False for simplicity, added notes on call cou…

a971f37

…nts, added test for when batch log is off

thomasjpfan reviewed Feb 13, 2020

View reviewed changes

fixed first_batch_ attr, added assert on log_metric call count when k…

b30cd3a

…eys are ignored

thomasjpfan approved these changes Feb 14, 2020

View reviewed changes

added docstring and test for first_batch_ attribute

bc763d8

Merge branch 'master' into master

bb46abd

thomasjpfan approved these changes Feb 15, 2020

View reviewed changes

jakubczakon added 2 commits February 16, 2020 13:29

formatting changes

779fb86

Merge branch 'master' of https://github.com/neptune-ai/skorch

7cff45f

BenjaminBossan approved these changes Feb 16, 2020

View reviewed changes

BenjaminBossan merged commit dc70fb4 into skorch-dev:master Feb 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Neptune logging #586

Added Neptune logging #586

jakubczakon commented Feb 3, 2020

BenjaminBossan commented Feb 3, 2020

jakubczakon commented Feb 3, 2020

BenjaminBossan commented Feb 3, 2020

jakubczakon commented Feb 4, 2020

BenjaminBossan commented Feb 4, 2020

BenjaminBossan left a comment

BenjaminBossan Feb 6, 2020

BenjaminBossan Feb 6, 2020

BenjaminBossan Feb 6, 2020

BenjaminBossan Feb 6, 2020

BenjaminBossan Feb 6, 2020

jakubczakon Feb 7, 2020 •

edited

Loading

BenjaminBossan Feb 6, 2020

jakubczakon Feb 7, 2020

jakubczakon commented Feb 7, 2020

BenjaminBossan left a comment

jakubczakon commented Feb 10, 2020

BenjaminBossan commented Feb 10, 2020

jakubczakon commented Feb 11, 2020

BenjaminBossan commented Feb 11, 2020

jakubczakon commented Feb 12, 2020

thomasjpfan Feb 13, 2020

BenjaminBossan Feb 13, 2020

jakubczakon Feb 14, 2020 •

edited

Loading

jakubczakon Feb 14, 2020

BenjaminBossan Feb 14, 2020

thomasjpfan Feb 14, 2020

BenjaminBossan Feb 14, 2020

thomasjpfan Feb 13, 2020

jakubczakon Feb 14, 2020

thomasjpfan Feb 14, 2020

jakubczakon commented Feb 15, 2020

jakubczakon commented Feb 16, 2020

BenjaminBossan left a comment

	experiment object or subclass the `on_*`` methods.
	experiment object or subclass the ``on_*`` methods.

	user.
	user, keys such as those starting with 'event_' or ending on
	'_best' are ignored by default.

Added Neptune logging #586

Added Neptune logging #586

Conversation

jakubczakon commented Feb 3, 2020

BenjaminBossan commented Feb 3, 2020

jakubczakon commented Feb 3, 2020

BenjaminBossan commented Feb 3, 2020

jakubczakon commented Feb 4, 2020

BenjaminBossan commented Feb 4, 2020

BenjaminBossan 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

Choose a reason for hiding this comment

jakubczakon Feb 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubczakon commented Feb 7, 2020

BenjaminBossan left a comment

Choose a reason for hiding this comment

jakubczakon commented Feb 10, 2020

BenjaminBossan commented Feb 10, 2020

jakubczakon commented Feb 11, 2020

BenjaminBossan commented Feb 11, 2020

jakubczakon commented Feb 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubczakon Feb 14, 2020 • 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

jakubczakon commented Feb 15, 2020

jakubczakon commented Feb 16, 2020

BenjaminBossan left a comment

Choose a reason for hiding this comment

jakubczakon Feb 7, 2020 •

edited

Loading

jakubczakon Feb 14, 2020 •

edited

Loading