Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple results in NeptuneObserver completed_event() #68

Merged
merged 3 commits into from
Nov 18, 2019
Merged

Support multiple results in NeptuneObserver completed_event() #68

merged 3 commits into from
Nov 18, 2019

Conversation

david1309
Copy link
Contributor

When using sacred the result variable can be either a float (which NeptuneObserver code currently supports) or multiple results (a tuple, which causes a crash do to neptune.log_metrics() not supporting tuples).

This is a quick fix I made ... but it would be nice that if one of the elements of the tuple is object (e.g. If parts of my results is a buffer / dataset) I could also store it

@kamil-kaczmarek
Copy link
Contributor

@david1309 thanks!

for i, r in enumerate(result):
if isinstance(r, float):
neptune.log_metric(f'result_{i}', r)
else: # Ignore non float results # TODO: Find a way of logging objects (log_artifact maybe??)
Copy link
Contributor

@jakubczakon jakubczakon Nov 14, 2019

Choose a reason for hiding this comment

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

Yes @david1309, you can log objects either via log_artifact on a path to a pickled object or you could use neptunecontrib.monitoring.utils.pickle_and_send_artifact(obj, 'name.pkl') which pickles to a temporary path and sends that to Neptune.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know, as some objects I want to store don't actually have a path / pickle associated to them, so this pickle_and_send_artifact is a nice wrapper

neptunecontrib/monitoring/sacred.py Outdated Show resolved Hide resolved
@jakubczakon
Copy link
Contributor

Thanks a lot @david1309 for PR-ing this.

BTW, any other ideas that could help you with Sacred+Neptune?

changed from formatting with f-strings to ` .format(i)` + using `pickle_and_send_artifact()`for logging objects

**Issue**:  if the result is a single result and its an object, the initial  `if result:` will evaluate to `false` and nothing will be logged
@jakubczakon
Copy link
Contributor

I am so sorry @david1309 but it seems that there is some wrong formatting in that sacred.py file.

Could you please make sure that the formatting as specified by pylint is ok.
Just go:

pip install -q -r test_requirements.txt

and then

python -m pylint -j 0 -f parseable neptunecontrib tests

I know it wasn't written anywhere other than in .travis.yml and I apologize for that.
I will add those instructions to the readme to make sure contributing is easy.

fixed formatting
@jakubczakon jakubczakon merged commit f5f78b8 into neptune-ai:master Nov 18, 2019
@jakubczakon
Copy link
Contributor

@david1309 figured I'd help with the formatting.
Thanks again for the PR!

@david1309 david1309 deleted the patch-1 branch November 18, 2019 14:44
@david1309
Copy link
Contributor Author

david1309 commented Nov 18, 2019

Glad it worked, thanks for the formatting.

Btw, what I said about "if the result is a single result and its an object, the initial if result: will evaluate to false and nothing will be logged" ... I'm not sure it actually happens, there might be no bug at all :)

Regarding any other improvements for Neptune - sacred integration tbh I don't have any suggestions atm since I haven't use these tools in integration too much ... i'll let you know if I think of something else

@kamil-kaczmarek
Copy link
Contributor

@david1309 @jakubczakon thanks for this contribution :)

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.

3 participants