Skip to content

add StaticLayer.crop() to match DynamicLayer API - #44893

Open
ai-man-codes wants to merge 3 commits into
huggingface:mainfrom
ai-man-codes:fix/static-layer-crop
Open

add StaticLayer.crop() to match DynamicLayer API#44893
ai-man-codes wants to merge 3 commits into
huggingface:mainfrom
ai-man-codes:fix/static-layer-crop

Conversation

@ai-man-codes

Copy link
Copy Markdown

What does this PR do?

Fixes #43011

StaticLayer was missing a .crop(max_length) method, so implemented that according to the discussion of the issue.

Added StaticLayer.crop(max_length) to match the API of StaticCache with the DynamicCache

Before submitting

@ai-man-codes

Copy link
Copy Markdown
Author

@zucchini-nlp I implemented StaticLayer.crop() method, can you review if this is implemented correctly? Thank You.

@qgallouedec

Copy link
Copy Markdown
Member

Thanks @ai-man-codes the core is right; three things before we can merge:

  • Negative max_length should mean "remove |max_length| tokens", as in DynamicLayer.crop — that's the API parity the issue asks for, and what assisted decoding uses. Right now it raises instead
  • StaticSlidingWindowLayer inherits this and breaks: it keeps a second python counter (cumulative_length_int) that stays stale, and once the window is full the states are rolled, so [max_length:] isn't the tail. Needs an override that refuses to crop past the window, like DynamicSlidingWindowLayer.
  • A test

Also needs a rebase: get_max_cache_shape is get_max_length now.

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.

StaticLayer cache layer to implement .crop(seq_len) to match API of DynamicLayer

3 participants