Add utilities functions to time functions #286
Conversation
dask_ml/utils.py
Outdated
| """ | ||
| Output execution time of a function to the given logger level | ||
|
|
||
| :param str name: How to name the timer (will be in the logs) |
There was a problem hiding this comment.
We use numpydoc style docstrings. Could you update these to use a Parameters section (don't need ad Returns).
dask_ml/utils.py
Outdated
| stop = time.perf_counter() | ||
| delta = datetime.timedelta(seconds=stop - start) | ||
| _logger_level = getattr(_logger, level) | ||
| _logger_level("Finished %s in %s", name, delta) # nicer formatting for time. |
There was a problem hiding this comment.
How is this formatted for a timedelta?
There was a problem hiding this comment.
>>> import logging
>>> import datetime
>>> logging.warning("%s", datetime.timedelta(seconds=50))
WARNING:root:0:00:50
>>> logging.warning("%s", datetime.timedelta(seconds=500))
WARNING:root:0:08:20
>>> logging.warning("%s", datetime.timedelta(seconds=5000))
WARNING:root:1:23:20
There was a problem hiding this comment.
Sorry for the delay @gglanzani.
The _timer context manager is what I had in mind. I'm not sure whether the _timed decorator will be useful or not. IIRC, there aren't as many place where we would log the entire function execution time, though I may be long. Could you look at a few places where we currently log the time, and replace them with _timer?
|
@TomAugspurger There are usages of |
|
Pushed some formatting changes. The usages of |
dask_ml/utils.py
Outdated
| level : str | ||
| On which level to log the performance measurement | ||
| """ | ||
| start = time.perf_counter() |
There was a problem hiding this comment.
Looks like time.perf_counter is new in python 3.3, and we support 2.7.
There was a problem hiding this comment.
Yeah, I fixed that. I'm now checking (locally) why the check_estimator test is so slow.
There was a problem hiding this comment.
I've noticed some strangeness with that test previously. If necessary, we can disable it, but I'm a bit concerned that it's showing this change has a performance impact (though I don't see why that would be).
There was a problem hiding this comment.
@TomAugspurger I'm running it locally with the decorator turned off, but I don't see different results.
I'll test a bit more and report back.
There was a problem hiding this comment.
@TomAugspurger I forgot an indentation in the Lloyd loop :) Fixed now
There was a problem hiding this comment.
It's entirely possible that was my fault when I did the reformatting. If so then you have my apologies :)
|
@TomAugspurger I've rebased against master. Local build is green, let's see what py27 holds for me. |
|
All green, thanks @gglanzani! |
Reference #193