Wrong Numba Cache Directory

[seanlaw]

@NimaSarajpoor Had asked:

I think we should open an issue for #1067 (comment). This can help us to track it separately. What do you think?

This is the corresponding ticket in response to his question.

From what I understand, once a package is installed, the nbc/nbi files are typically written to the site-packages/stumpy/__pycache__ subdirectory if a user imports stumpy. However, specifically for pytest, cache files are written locally in stumpy/__pycache__ because pytest isn't using the installed package and, instead, are using the files from the local stumpy/*py! So, when a user uses stumpy, caching occurs in site-packages and all is well (since cache._clear() targets site-packages/stumpy/__pycache__. However, for unit testing, we get a different behavior because pytest is basically running things locally (i.e., it is creating the bytecode locally and not from site-packages) and accessing the bytecode/cache from stumpy/__pycache__ rather than from site-packages/stumpy/__pycache__. In other words, pytest isn't using the installed stumpy and, instead, pytest is using/importing local stumpy and creating the bytecode locally (independent of the installed package in site-packages).

So, it's a bit misleading when we do ./setup.sh && ./test.sh 😢

Note, that this (cache directory) behavior should NOT be observed from an end user's standpoint (i.e., when stumpy is installed and accessed from site-packages) and should only affect unit testing.

[seanlaw]

To accommodate pytest, I think we can modify cache._clear() to:

def _clear(cache_dir=None):
    """
    ...
    """
    if cache_dir is not None:
        numba_cache_dir = cache_dir
    elif "PYTEST_CURRENT_TEST" in os.environ:
        numba_cache_dir = "./stumpy/__pycache__"
    else:
        site_pkg_dir = site.getsitepackages()[0]
        numba_cache_dir = site_pkg_dir + "/stumpy/__pycache__"
    [f.unlink() for f in pathlib.Path(numba_cache_dir).glob("*nb*") if f.is_file()]

We'll have to modify some of the other functions as well to add the cache_dir parameter. I've tested it and it seems to work.

[seanlaw]

Additionally, when we call cache._save(), should we also do cache._clear() as well?

I think the unit test should look like this:

def test_cache_save_after_clear():
    cache.clear()
    cache.save()

    T = np.random.rand(10)
    m = 3
    stump(T, m)

    ref_cache = cache._get_cache()

    cache.clear()
    # testing cache._clear()
    assert len(cache._get_cache()) == 0

    cache.save()
    stump(T, m)
    comp_cache = cache._get_cache()

    # testing cache._save() after cache._clear()
    assert sorted(ref_cache) == sorted(comp_cache)

    cache.clear()

[NimaSarajpoor]

@seanlaw

specifically for pytest, cache files are written locally in stumpy/__pycache__ because pytest isn't using the installed package and, instead, are using the files from the local stumpy/*py

However, for unit testing, we get a different behavior because pytest is basically running things locally (i.e., it is creating the bytecode locally and not from site-packages) and accessing the bytecode/cache from stumpy/__pycache__ rather than from site-packages/stumpy/__pycache__.

Thank you for the clear explanation!

should we also do cache._clear() as well?

I think it is good idea to enclose this test function by cache._clear(), i.e. one in the beginning and one at the end. We do something similar in ./test.sh, where we do clean_up() in the beginning as well as at the end. We do not have any other test that results in writing data into numba cache directory, but I think it is still worth it to have cache._clear() in the beginning. This will give us a better peace of mind.

---

Question:
Do we need to add a check to make sure ref_cache is not empty?

We noticed this issue recently where both ref and comp were empty as we were reading from the location that was different than the "write" location. Similarly, in the future, if some code changes, that can accidentally result in getting an empty list here. And the assertion will still pass! it is like bypassing the test. Am I trying to be TOO careful here? 😅

[seanlaw]

Do we need to add a check to make sure ref_cache is not empty?
We noticed this issue recently where both ref and comp were empty as we were reading from the location that was different than the "write" location. Similarly, in the future, if some code changes, that can accidentally result in getting an empty list here. And the assertion will still pass! it is like bypassing the test. Am I trying to be TOO careful here? 😅

I considered this as well. However, the tricky part is that we still need to pass coverage testing where no cache files will ever be written. I did not want to be distracted by this yet until the other stuff was done. Perhaps, now is the time to think about it :)

[seanlaw]

Perhaps, now is the time to think about it :)

Done! Please see PR #1070

[NimaSarajpoor]

However, the tricky part is that we still need to pass coverage testing where no cache files will ever be written.

Right! We are not omitting it from the coverage testing.

---

I feel the underlying challenge is "how can we make sure we are reading data from the correct location to which data was written?" So, can we try to track that numba cache directory by defining a config var like:
config.NUMBA_CACHE_DIR which can be set to None as default. Then, if cache._save() is called, we can write the directory there. We can even raise warning if the user provided numba cache directory is different than an already-set config. config.NUMBA_CACHE_DIR

[seanlaw]

@NimaSarajpoor Since we are talking about code, let's carry on the conversation over in PR #1070. Otherwise, it gets confusing needing to jump back and forth.