Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Handle past_key_values AttributeError in generate Function #137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

scifisatan
Copy link

Issue

The generate function in app_janusflow.py encountered the following error:

AttributeError: 'tuple' object has no attribute 'get_seq_length'

image

I was running Python 3.10.12 and it error occured when i ran python3 demo/app_janusflow.py

Cause:
The error occurred because past_key_values was being passed as a tuple to vl_gpt.language_model.model, while the expected structure required either None or a properly formatted past key-value state.

Fix

  • Ensured that past_key_values is correctly initialized as None at the start (step == 0).
  • Properly handled its reassignment in subsequent steps:
    if step == 0:
        past_key_values = None  # Ensure it starts as None
    else:
        past_key_values = tuple(past_key_values) if past_key_values else None  # Convert only if it's valid
  • This prevents the tuple from being incorrectly accessed when past_key_values is None.

Testing

  • Verified that the model runs without throwing an AttributeError.
  • Checked inference results to ensure correctness.

@scifisatan
Copy link
Author

scifisatan commented Feb 4, 2025

#77
#99
fixes these issue

@KCFindstr
Copy link

Could you elaborate how this fixes the issue? past_key_values hasn't been assigned any values inside the loop so it is always None, or did I miss anything?

@scifisatan
Copy link
Author

Yeah, so the main issue was that past_key_values was never getting updated inside the loop, which meant it was always None. That’s why the model couldn’t reference past states properly, leading to the AttributeError.

The fix ensures that at step == 0, past_key_values starts as None, but in later steps, it actually stores the past states from the model’s output:

@KCFindstr
Copy link

I don't see where you store the past states from the model's output in this fix - past_key_values is still None after the first step. Are you missing a line similar to past_key_values = outputs.past_key_values?

@nv-samcheng
Copy link

I don't think which needed to fix since kvcache doens't be used in JanusFlow

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.

3 participants