Neptune integration #648
Neptune integration #648
Conversation
|
It seems that the failed tests are related to Not sure how to proceed. |
pytorch_lightning/logging/neptune.py
Outdated
| try: | ||
| import neptune | ||
| except ImportError: | ||
| raise ImportError('Missing neptune package. Run pip install neptune-client') |
tests/test_logging.py
Outdated
| """Verify that basic functionality of neptune logger works.""" | ||
| tutils.reset_seed() | ||
|
|
||
| try: |
tests/test_logging.py
Outdated
| """Verify that pickling trainer with neptune logger works.""" | ||
| tutils.reset_seed() | ||
|
|
||
| try: |
pytorch_lightning/logging/neptune.py
Outdated
| self.experiment.set_property(key, value) | ||
|
|
||
| @rank_zero_only | ||
| def append_tags(self, tag, *tags): |
There was a problem hiding this comment.
this tag/tags as two variables sound quite strange, rather make it generic and just single var
There was a problem hiding this comment.
The idea here is to support all the following options:
logger.append_tags('resnet')
logger.append_tags('resnet', 'augmentations')
logger.append_tags(['resnet','augmentations'])But I think for the simplicity I will go with the last option (list).
There was a problem hiding this comment.
well you can make a simple fork via using single variable:
def append_tags(self, tags):
if not isinstance(tags, (list, set, tuple)):
tags = [tags] # make it as an iterable is if it is not yet
...
pytorch_lightning/logging/neptune.py
Outdated
|
|
||
| :param str log_name: The name of log, i.e. bboxes, visualisations, sample_images. | ||
| :param str|PIL.Image|matplotlib.figure.Figure image: The value of the log (data-point). | ||
| Can be one of the following types: PIL image, matplotlib.figure.Figure, path to image file (str) |
There was a problem hiding this comment.
add spaces at the beginning, so it is assigned to the paramaters
There was a problem hiding this comment.
this is so more parameters in the docstring...
There was a problem hiding this comment.
I think added spaces wherever needed.
Do you mean I should refactor passing neptune-specific parameters as one dictionary or something like that?
pytorch_lightning/logging/neptune.py
Outdated
| Requires either an API Key (online mode) or a local directory path (offline mode) | ||
|
|
||
| :param str|None api_key: Required in online mode. Neputne API token, found on https://neptune.ml. | ||
| Read how to get your API key here https://docs.neptune.ml/python-api/tutorials/get-started.html#copy-api-token. |
There was a problem hiding this comment.
add spaces at the beginning, so it is assigned to the parameters
|
To the failing test, pls try increase number of training epochs, like a double... |
|
Ok on it (failing test) @Borda |
|
3 travis jobs worked but 2 still fail. |
|
Locally it passes and in those 3 other travis jobs it passes as well. |
|
I have one hypothesis. Default checkpoint is initialized with |
|
I have had the similar issue in #653 and setting |
|
Thank you @kuynzereb and @Borda it worked! |
true, in theory, it may happen that the next epoch does not lower training/validation loss as the loss is usually oscillating and the test case is to be minimal in terms of time and computational power... |
|
Hi there @Borda, happy new years! I was wondering if there is anything else I could do to help merge this PR? |
|
@jakubczakon nop, it is good from my side, Nice work! |
|
@jakubczakon great addition! |
Before submitting
What does this PR do?
Creates integration described in #642
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
I sure did :)
Note
I had trouble finding how to set up local docs so I am not sure if those work property.
Also, I added note about logging to readme which links to docs which I am quite sure are not created.
So I presume there will be trouble here.