Skip to content

Conversation

@hturki
Copy link

@hturki hturki commented Oct 25, 2025

fixes #20

@ki-lw
Copy link

ki-lw commented Oct 25, 2025

Thanks for your modification! However, I believe there’s a logical issue with the change.

The cache zeroing occurs under if not self.global_sink, meaning it only happens when global_sink is False. In that case, the sink region is not global — otherwise, we could simply set global_sink = True.

During the subsequent cache update process, whether the region cache["k"][:, self.generator.model.config.sink_size * self.frame_seq_length:] was zeroed beforehand doesn’t really matter. It will be recomputed based on current_start and write_len, thus properly updated.

In my view, the if self.global_sink branch mainly concerns the update mechanism for the sink region, and zeroing [sink_size * self.frame_seq_length:] does not affect the subsequent updates of that region itself.

@hturki
Copy link
Author

hturki commented Oct 26, 2025

Good point! Looking at this more closely, I actually think that the zero'ing out is unnecessary since as you mention the cache entries will be overwritten during the subsequent recompute. I think that the proper fix is to actually modify https://github.com/NVlabs/LongLive/pull/21/files#diff-4ed558c68c6ec9a180de74af9d37373fdf8fff84ac40cbf34a6669ad03599a18R260 to account for whether we are using a global sink or not.

@hturki
Copy link
Author

hturki commented Oct 26, 2025

This is what I get with the model shared on hugging face before this MR is applied

rank0-0-0_lora.mp4

And then with the fix, this is what we get with the global sink enabled:

rank0-0-0_lora.mp4

And this is with the fix + the global sink disabled:

rank0-0-0_lora.mp4

At least on this example, it seems that keeping the global sink is important for maintaining the appearance of the boy.

@ki-lw
Copy link

ki-lw commented Oct 26, 2025

Yeah! That’s great!

In fact, based on the experimental setup described in the LongLive paper, the global_sink setting should default to True. I was just a bit surprised to see that in the provided longlive_interactive_inference.yaml, it’s set to False.

That said, your modification effectively fixes the behavior when global_sink is set to False — even though none of us would really recommend using that configuration (haha).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

global_sink: false causes permanent KV cache corruption

2 participants