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

Investigate functools.lru_cache #1125

Open
jmeyers314 opened this issue Jun 19, 2021 · 1 comment
Open

Investigate functools.lru_cache #1125

jmeyers314 opened this issue Jun 19, 2021 · 1 comment
Labels
cleanup Non-functional changes to make the code better

Comments

@jmeyers314
Copy link
Member

I'm not sure if functools.lru_cache existed when we added galsim.utilities.LRU_Cache, but I've been playing around with it recently and I think I like it better. With this simple program:

from functools import lru_cache
from galsim.utilities import LRU_Cache
import numpy as np

@lru_cache(1024)
# @LRU_Cache
def f(x):
    return x*2+np.sin(3*x)+np.cos(5.5*x+0.2*x)**2

def g(x):
    return f(x) + f((x+31)%1000)

def main():
    for i in range(1000000):
        g(i%1000)

if __name__ == '__main__':
    main()

I get better performance with functools.lru_cache (~1.0 s vs ~1.8 s), and also like the output I get through gprof2dot better:

utilities.LRU_Cache
galsim_utilities_LRU_Cache

functools.lru_cache
functools_lru_cache

Querying the cache hits/misses is also nice with functools.lru_cache.

The one feature we'd be giving up is the ability to resize the already-created cache. It's definitely nice for the user to be able to set the size, so I think we'd need to invent some kind of API wrapper for that. Looks like there's a __wrapped__ attribute that might help.

Finally, functools.lru_cache is only available for python >3.2, so this would be easiest if we dropped support for 2.7.

@rmjarvis
Copy link
Member

Agreed. I looked into this a bit for TreeCorr (where I already dropped 2.7 support on main), and for TreeCorr, I really do want the resize option, so I decided against switching there. But for GalSim, I was thinking we didn't really need that for the cases where we use this, so I'd be fine with dropping the ability to resize and switch over.

Or, if you feel like trying to figure out how to shoehorn the resizing in, that's also good by me.

@rmjarvis rmjarvis added the cleanup Non-functional changes to make the code better label Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Non-functional changes to make the code better
Projects
None yet
Development

No branches or pull requests

2 participants