Won't we need to check both attributes for a certain time? AFAICS the old DynamicCache instance doesn't have a layers attribute.

Yes, true, but while looking at this, I found another issue. Namely, we first initialize past_key_values.cross_attention_cache = DynamicCache() and then iterate over its key_cache / layers, but of course those are going to be empty. I confirmed this when running the tests. So, technically, this line is a no-op.

Now I wonder: can it be removed or should we save the is_updated from before we override cross_attention_cache with the empty DynamicCache()? When checking the attribute before, it is not empty but can have values like {0: True, 1: True}.

@zucchini-nlp I think something was messed up in #2096 to end up in this situation.

We had a major refactor on cache recently, and prob the new format isn't 100% backward compatible. Can you remind me what was the desired behavior for prefix-tuning on encoder decoder models. We append virtual tokens to decoder inputs only or to encoder ids as well?

I just checked the prefix tuning paper, the idea is to have separate prefix tokens (or rather, embeddings) for both encoder and decoder.

Ahh, I see, Overwriting cross_attention_cache actually won't change the values of is_updated afaik and thus we still have to set them to False in order to save cross-encoder cache

Maybe we can just iterate over cache.is_updated and change it in-place, so we don't have to infer number of layers from cache.layers, wdyt?

Ah, good suggestion. Just so I'm sure I understand correctly, currently we have:

                past_key_values = EncoderDecoderCache.from_legacy_cache(past_key_values)
                past_key_values.cross_attention_cache = DynamicCache()
                past_key_values.is_updated = {
                    layer_idx: False for layer_idx in range(len(past_key_values.cross_attention_cache.layers))
                }

which makes no sense because past_key_values.is_updated will always end up being an empty dict. Instead, we should do this, right?

                past_key_values = EncoderDecoderCache.from_legacy_cache(past_key_values)
                past_key_values.cross_attention_cache = DynamicCache()
                for key in past_key_values.is_updated.keys():
                    past_key_values.is_updated[key] = False

And this should also work with older transformers versions.

yep, an empty dict would throw errors so we need an explicit False