mlco2 / codecarbon

Track emissions from Compute and recommend ways to reduce their impact on the environment.
https://mlco2.github.io/codecarbon
MIT License
1.11k stars 173 forks source link

Fixed duration values in task monitoring #649

Closed Gommarlo closed 3 weeks ago

Gommarlo commented 4 weeks ago

Fixed a bug when measuring task duration: formerly it used epoch time as a start value, while every other instance of time measurement uses relative time. This resulted in inconsistent values.

Replacing time.time() with time.perf_counter() solves the issue.

benoit-cty commented 4 weeks ago

Thanks for reporting this. Do you think it will be better to also use it for https://github.com/mlco2/codecarbon/blob/f16c986aa7cb52f6b4e7439fbc100691a1339432/codecarbon/core/measure.py#L28 And https://github.com/mlco2/codecarbon/blob/f16c986aa7cb52f6b4e7439fbc100691a1339432/codecarbon/core/measure.py#L43

Gommarlo commented 3 weeks ago

My suggestion here is to not use time.time() at all for multiple reasons:

Another thing to keep in mind, is that the fix I provided might not be the ideal one: time.perf_counter() is the highest precision you can get when measuring time (along with time.perf_counter_ns()) which implies a larger overhead when accessing the clock.

Another option is to use time.monotonic() which has significantly less overhead, does not have time.time()'s issues, but also has a lower precision.

Depending on your requirements you should use whichever you see fit.

In my humble opinion, I believe that time.monotonic() should be the standard duration tracker due to Codecarbon's goal of being as lightweight as possible, but for tasks it might be helpful to have higher precision as I may want to run short energy-intensive code (or you could allow the user to choose which timer to use)

benoit-cty commented 3 weeks ago

This will be the 2.7.0 release of CodeCarbon.