search

drphilmarshall / Pangloss

Line of sight mass reconstruction in the Universe
GNU General Public License v2.0
10 stars 10 forks source link

timeit: units, garbage collection #75

Closed drphilmarshall closed 8 years ago

drphilmarshall commented 9 years ago

I read with interest at https://docs.python.org/2/library/timeit.html that a) the unit of time that the timeit default timer returns may not be millisec and b) timeit may turn off garbage collection while it runs! I think this means that a) we need to read the manual carefully before drawing conclusions about absolute run times and b) timing needs to be an option that is off by default but that we can turn on if we want to do any benchmarking. Interested to see if this fixes the memory leak by allowing the del commands to function again.

drphilmarshall commented 9 years ago

I think the memory leak is still with us, no? In which case, @sweverett you can reassign the milestone of this issue to "Post-SULI Clean-up" and then - da da-da dah! - close the "End of SULI Program" milestone :-)

sweverett commented 9 years ago

Yes, the memory leak is most definitely still with us. I'll reassign now. And it'll be tough - but I'll close the milestone! I'm on a mini-vacation with my family right now but will start some of the post-SULI cleanup when I get back.

sweverett commented 8 years ago

To address your issues: -The default unit for timeit is seconds not milliseconds, but I was aware of this when reporting numbers to you and in my report. -I added a time option in background.py to turn on/off time recording similar to vb. It is set to True right now since it is helpful for keeping track how code changes alter the runtime. -timeit does turn off garbage collection! But this ended up not being the cause of the memory leak that initiated this issue. I tracked the memory usage of lens_by_halos() with time on and off and there was no difference, so I think we're safe.

drphilmarshall commented 8 years ago

Great! Thanks Spencer.

On Wed, Apr 13, 2016 at 12:50 PM, Spencer Everett notifications@github.com wrote:

To address your issues: -The default unit for timeit is seconds not milliseconds, but I was aware of this when reporting numbers to you and in my report. -I added a time option in background.py to turn on/off time recording similar to vb. It is set to True right now since it is helpful for keeping track how code changes alter the runtime. -timeit does turn off garbage collection! But this ended up not being the cause of the memory leak that initiated this issue. I tracked the memory usage of lens_by_halos() with time on and off and there was no difference, so I think we're safe.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/drphilmarshall/Pangloss/issues/75#issuecomment-209619927