[core / modeling] Fix training bug with PEFT + GC

[younesbelkada]

What does this PR do?

Fixes #28023

4.36.0 led to a bug when users are in the case of GC + training which should force-set use_cache to False here: https://github.com/huggingface/transformers/blob/main/src/transformers/models/llama/modeling_llama.py#L1008 - which is force-set to True during the backward pass for some reason, only in the case where one uses PEFT + GC.

The fix is to force-set use_cache to False before computing past_key_value_length here: https://github.com/huggingface/transformers/blob/main/src/transformers/models/llama/modeling_llama.py#L1042

cc @amyeroberts

[amyeroberts]

Thanks for fixing!

[amyeroberts]

I don't see why moving the logic here should make a difference?

[younesbelkada]

It is very unusual but apparently when using GC + training the backward pass calls again the forward pass but with a non-None past_key_value.
I think use_cache is set to True by default on all configs; therefore avoiding the block

        if use_cache:
            use_legacy_cache = not isinstance(past_key_values, Cache)
            if use_legacy_cache:
                past_key_values = DynamicCache.from_legacy_cache(past_key_values)
            past_key_values_length = past_key_values.get_usable_length(seq_length)

to be called seems to be the right fix. I will add tests directly on PEFT as it is related to PEFT

[ArthurZucker]

would be nice to also get why we have to do this (why the forward re-runs) but static cache might actually do the same

[DailyCasual]

I am having this issue with Kohya Dreammaker as well. Downgrading to 4.27.1 has seemed to help. How do I go about actually fixing this issue? I have all the pre reqs installed and my models work, but I have no idea how to use python to fix this issue?

[amyeroberts]

@DailyCasual Could you open a new issue, detailing the error encountered, what you've experimented e.g. versions and their behaviour? This helps us better track what bugs are new and have been resolved.