cache/nocache: formula with slice operator and forecast date not correctly updated

[EngieAntoine]

With the slice operator (which redefine from_value_date and to_value_date), the cache don't work correctly. For example, if we have a slice with todate=(today), the cache will cut value_date after this todate (which become the revision_date). Here the unit test (seem I lost the right to push something here > 403):

def test_cache_slice(tsa):
    days = list(range(1, 10))
    # Define series
    series_name = 'forecast'
    series = pd.Series(
        [day for day in days],
        index=[pd.Timestamp(f'2020-01-{day:02d}') for day in days]
    )
    tsa.update(
        series_name,
        series,
        'test_cache_slice',
        insertion_date=pd.Timestamp('2020-01-01')
    )
    # Define formula
    formula_name = 'sliced-forecast'
    tsa.register_formula(
        formula_name,
        f'(slice (series "{series_name}") #:todate (today))',
    )
    # Define cache policy
    policy_name = 'daily-cached'
    tsa.new_cache_policy(
        policy_name,
        initial_revdate='(date "2020-01-05")',
        look_before='(shifted now #:days -2)',
        look_after='(shifted now #:days 10)',
        revdate_rule='0 12 * * *',
        schedule_rule='0 12 * * *',
    )
    tsa.set_cache_policy(
        policy_name,
        [formula_name]
    )
    # Initiate policy
    cache.series_policy_ready(tsa.engine, formula_name, namespace=tsa.tsh.namespace)
    cache.set_policy_ready(
        tsa.engine,
        policy_name,
        True,
        namespace=tsa.tsh.namespace
    )
    cache.refresh_series(tsa.engine, tsa, formula_name)
    # Check
    pd.testing.assert_series_equal(
        tsa.get(formula_name),
        tsa.get(formula_name, nocache=True)
    )

[zogzog]

Thanks for the report. It's odd that you can't push, I just checked and you still have write access (and I didn't change anything).

[EngieAntoine]

I finally succeed to push on [test_autotrophic_cache_refresh] branch

[Bernouilly]

Hi! I guess the point is that the test is failing ? If so it should appears in it (with a context manager to catch the error) Also the trace of the cached and no-cached series should also be exposed.

Or maybe I am missing something?

[EngieAntoine]

no the point is that this test must not failed and a fix must be done for this.

[Bernouilly]

OK, then, but even to enlight a bug, the test must be green at each commit. This is why we write something like that: https://github.com/pythonianfr/tshistory_refinery/blob/b3614e428fe4ec825caf34e7334b144b6e511b03/test/test_api.py#L630

And the next commit, that fix the bug will also change the test to remove the error catching mechanism.

Furthermore, the test should ideally show what kind of difference there are between the two series

[zogzog]

Indeed slice + today creates, even for the formula whose underlying series has only 1 rev, a formula with infinitely many latent revisions. What you are saying is that the cache should make these revisions explicit (according to the revdate rule). I think this can by tackled by detecting the presence of, at least initially, slice + a today expression, and imperatively computing all the versions.

[Bernouilly]

For my curiosity: are you trying to build some pseudo-versioning on a non-versioned series?

[EngieAntoine]

heu no. Just trying to understand why some Cronos formulas don't be refresh correctly in the cache system. And this is the case for most of them because using the slice operator with to_value_date=today

[zogzog]

Out of curiosity, what is the purpose of those slices ? Is Adrien tweaking some priorities ?

[EngieAntoine]

I don't know. Have to check with him : it's existing series.

by the way, what do you mean by "this can by tackled by detecting the presence of, at least initially, slice + a today expression, and imperatively computing all the versions." ? how can we do that ?

[zogzog]

by the way, what do you mean by "this can by tackled by detecting the presence of, at least initially, slice + a today expression, and imperatively computing all the versions." ? how can we do that ?

In refresh_series we prune the insertion dates range using reduce_frequency. We probably want to skip this step if there is a slice + today. We have the formula at hand. A quick and dirty check would be to do a substring check (e.g. "(today)" in formula). A tree walk would be more robust.

[zogzog]

I have pushed a proposed fix in your branch.

[EngieAntoine]

ok thks. I will test it asap.

[EngieAntoine]

so, it's works in most case... but on insertion date of today, I've got a valuedate with cached version and not with non cached version.

[EngieAntoine]

so I confirmed that the correction for the slice is not correct : if we have a formula like : slice (cronos "77eb0f50-e3ce-4088-b3e0-d18001f6172c") #:todate (shifted (today) #:days -1)) it will not slice the formula and we will have values for today with the cached version.

[EngieAntoine]

by the way, I wonder if the problem don't come from the fact that (today) is not a real today but a now...

[zogzog]

Ok, interesting example, thanks.

[Bernouilly]

I am rephrasing to check my understanding:

In the last version of the cache you have a value date corresponding at "today", but you wanted the last 24 hours to be missing, am I right ?

Could you check with an .history() call that the last values were inserted in the last refresh of the cache?

[Bernouilly]

Is this cache running for a few days ? What are its parameters?

[Bernouilly]

By reading the code, both of singularity_formula and refinery.cache, I don't see a glaring error in the way we handle the bounds, and hence I don't have an explanation for this bug.

An access to a preprod or a test (difficult to write because of the autotrophic nature of the operator) is needed to go further on my side.

A few additional information might help, such as the result of a print of the cron_range in the tshistory_refinery.cache.refresh_series function

[zogzog]

Well, or a test that exhibits the issue (it will have to be written anyway).

[EngieAntoine]

here, the result of series that containsslice (cronos "77eb0f50-e3ce-4088-b3e0-d18001f6172c") #:todate (shifted (today) #:days -1)):

I will try to make an unit test to exhibits the issue tomorrow

[pitchopp]

Hello, I pushed a new branch "bugfix/cached_slice" containing test_cache_slice2 which show that there is a bug with the slice and cache. Can you have a look at it please ?

[Bernouilly]

Hello Amin thank you for you contribution!

I have some things to say about your test, however, it does not seem to reproduce the bug described by Antoine

• first of all, same remarque than for Antoine: ALL THE TESTS MUST BE GREENS, EVEN WHEN THEY DESCRIBE A BUG

In you case, you could do something like:

    with pytest.raises(Exception) as error:
        pd.testing.assert_series_equal(
            tsa.get(formula_name),
            tsa.get(formula_name, nocache=True)
        )
    assert len(tsa.get(formula_name)) == 4
    assert len(tsa.get(formula_name, nocache=True)) == 5
    assert tsa.get(formula_name, nocache=True).index[0] not in tsa.get(formula_name).index

The context manager allows to catch the error, hence the test does not fail but show the problem. The following assertions specifiy the difference between the two series: one is shorter than the other and the FIRST value is missing.

• The case you described is highly pathological but, IMO, does not represent the case describe by Antoine in production. An easy fix in your case would be to chance the cache policy with:

look_before='(shifted now #:days -2)',. changed to look_before='(shifted now #:days -4)',

With this policy, your 2 series (with and without cache) would be strictly equals.