creating cache for faster access to results of previous cnmf extension calls

[clewis7]

adding file to extensions for a cache

cache should allow users to quickly access results of previously made calls to cnmf extensions that have the same function_name, args, and kwargs

[kushalkolar]

This is a great start! I'l implement the following methods:

clear()
set_maxsize()

all I can think of for now

[kushalkolar]

For use in the extensions, I'd create a separate Cache instance for each extension class. Both mcorr and cnmf have a get_output func name.

[clewis7]

will work on testing out cache for different cnmf and mcorr extension functions as well as checking reduced time in accessing!!

[kushalkolar]

almost there! :D

[kushalkolar]

also for some reason tests are failing for cnmfe, weird because they're passing on master and this branch has no diff with the tests on master. can check in details later.

[clewis7]

just did basic impl of using memory size, appears to be working when I test it out

wasn't completely sure how you wanted the bounds to work because the likelihood of the number you set for bytes being equivalent to the number that the output sizes in the cache will add up to be is unrealistic

can go back and alter impl so that if adding an item will cause cache to go over cache size, will remove the most recently used item...but this could also cause many items to be removed if removing simply a path for example will not necessarily cause a huge change to size of outputs but that was the most recently accessed item in the cache

[kushalkolar]

just did basic impl of using memory size, appears to be working when I test it out

wasn't completely sure how you wanted the bounds to work because the likelihood of the number you set for bytes being equivalent to the number that the output sizes in the cache will add up to be is unrealistic

can go back and alter impl so that if adding an item will cause cache to go over cache size, will remove the most recently used item...but this could also cause many items to be removed if removing simply a path for example will not necessarily cause a huge change to size of outputs but that was the most recently accessed item in the cache

Yes you are right!

I'm assuming you mean removing the least recently used, not most recently used.

Anyways there's a very simple way on this line 118 to make sure that it never exceeds the cache size! In general there's a more elegant way to implement your control flow here.

[kushalkolar]

I think that the reason the tests are failing is because cached values are being returned even though new args have been passed. I'm guessing this because there's a mismatch in the shape of input arrays which corroborates with the size of ixs_components:
https://github.com/nel-lab/mesmerize-core/runs/7168946258?check_suite_focus=true#step:7:748
https://github.com/nel-lab/mesmerize-core/runs/7168946258?check_suite_focus=true#step:7:1058.

[clewis7]

last commit message should be "...should now work..."

[kushalkolar]

I just realized, for things like get_spatial(), get_temporal() etc. we should always return a copy because otherwise the cached value would get changed if the user for example does stuff with the temporal downstream. We can have a default kwarg for all these cached functions return_copy=True. We should have the same thing for get_reconstructed() get_residuals() etc. but maybe default set as return_copy=False because these are larger arrays.

[clewis7]

I just realized, for things like get_spatial(), get_temporal() etc. we should always return a copy because otherwise the cached value would get changed if the user for example does stuff with the temporal downstream. We can have a default kwarg for all these cached functions return_copy=True. We should have the same thing for get_reconstructed() get_residuals() etc. but maybe default set as return_copy=False because these are larger arrays.

Will implement after I fix residuals and reconstructed movie extensions

[kushalkolar]

ok left to do:

1. Check that cnmf.C, A, b, f sum up to 90% the data for the entire cnmf.estimates object
2. I just realized, for things like get_spatial(), get_temporal() etc. we should always return a copy because otherwise the cached value would get changed if the user for example does stuff with the temporal downstream. We can have a default kwarg for all these cached functions return_copy=True. We should have the same thing for get_reconstructed() get_residuals() etc. but maybe default set as return_copy=False because these are larger arrays.

Is this still an issue?

(2) I still am not sure why the size of get_temporal_components() cannot be calculated by what I have because I have verified that the output is of type numpy.ndarray, however the error that is being casted is an attribute error claiming that type list has no attribute data...makes absolutely no sense to me because the if block that should calculate the size of a numpy.ndarray is completely being skipped and then the output is being recognized as a tuple which is certainly not the case

[kushalkolar]

yay other than my comments about the copy, just need to write tests for cache!

• test that the MB and GB cache size limits work, set something small for the tests like 100MB
• test that cached vals are returned only when not present in cache dataframe
• tests should be in a separate test function entirely, run a fresh batch of mcorr and cnmf in this function and tests that the cache stuff works here.

[kushalkolar]

So cache makes a big difference, reconstructed, residuals etc. movies are now FAST random-access scrollable. The majority of the time is spent in fetching data, the outer product doesn't take very long so we don't need to decorate get_reconstructed(), get_residuals() etc.. I tried and it actually slows it down considerably!

without_cache-2022-07-12_00.34.37.mp4

with_cache-2022-07-12_00.30.51.mp4

[kushalkolar]

I think it'd also be useful to have cache tests to make sure that it's returning the cached vals for the correct function and item (by checking the UUID).

[kushalkolar]

LGTM! Awesome job!

[kushalkolar]

Just a few minor things and I will merge! :D Test should pass on github after these changes!

[kushalkolar]

yay, test pass, going to merge