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

Caching decorator util #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Caching decorator util #2

wants to merge 1 commit into from

Conversation

cogreanu
Copy link
Collaborator

Created a decorator that can be used to cache the first value of instance method calls, without taking any parameters besides self into consideration. It is also possible to specify a "default value" to be returned after the first method call using a keyword argument.

While it is not possible to apply to every method that requires caches, it might be nice for simpler methods.

Before:

def method1(self, param):
  if self._method1_cache is None:
    return param + 2
  return self._method1_cache

def method2(self, param):
  if self._method2_cache is None:
    return param * 3
  return 1

After:

@cache
def method1(self, param):
  return param + 1

@cache(default_val=0)
def method2(self, param):
  return param * 3

It is possible to clear the cache of all values, or only for a subset of methods:

clear_decorator_cache() # clears everything
clear_decorator_cache([Node.method_1, Node.method_2])

Finally, while testing some circuits, numba complained about currently not supporting numpy 2.1 and above, which is why I updated the requirements.txt.

@jellevos
Copy link
Owner

Woah this is a really nice change! It would definitely make things more ergonomic! I have a few minor changes in mind but I suppose we should first try to use it to replace our current caching and see if we need to make any changes. Could you elaborate about numba and numpy? Where and when does it complain? I can help out with integrating this decorator :)

@cogreanu
Copy link
Collaborator Author

That's great to hear! What kind of changes do you think should be done?

About numba, when trying to run cardio.py for instance, I get the following error:

ImportError: Numba needs NumPy 2.0 or less. Got NumPy 2.1.

From what I could find online, the next numba release will support numpy 2.1, but I'm not sure when that will be out.

@jellevos
Copy link
Owner

About numba, when trying to run cardio.py for instance, I get the following error:

ImportError: Numba needs NumPy 2.0 or less. Got NumPy 2.1.

From what I could find online, the next numba release will support numpy 2.1, but I'm not sure when that will be out.

Ah I see! Sorry, I misunderstood as I was not aware of the use of numba, but I think it is used in the galois package. Thanks for updating the requirements.txt!

What kind of changes do you think should be done?

I think the only things I would change right now are the names, to make it really explicit what is happening. For example, we could call the decorator @locally_cached and change default_val to something like val_when_cached. Finally, we may want to rename the util folder to something like caching, because there is also some other code for caching to a local file, which we can move there.

I think that the most important thing now is to see if it is actually ergonomic by refactoring a small piece of code to use this cache. Let me know if I can help out with that!

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.

2 participants