InternVLChatModel's forward method lacks necessary img_context_token_id initialization

#4
by L-Yuan - opened

Description:
While exploring the code, I noticed a potential issue in the forward method, and I’d like to discuss it here for feedback and suggestions. Specifically:

  1. It seems that self.img_context_token_id is used in the forward method, but it’s not initialized within that method.
  2. The forward method is the only one that directly returns the attention output.
  3. It’s possible that users might call forward directly, which could lead to errors or unexpected behavior.

I understand that the img_context_token_id is set in other methods like batch_chat, chat, and generate, but this design seems to introduce a few risks:

  1. If forward is called directly, it may fail due to uninitialized variables.
  2. Tasks that depend on the attention output might be affected if this initialization is missed.

Possible Solutions:

  1. Would it be beneficial to initialize self.img_context_token_id within the __init__ method to ensure it’s always defined?
  2. Or should we add initialization logic to the forward method itself, similar to the other methods?
  3. Another idea might be to add validation checks to prevent the use of undefined tokens and ensure that everything is properly set up before execution.

I’m not entirely sure about the best approach, and I’d really appreciate any feedback or insights from the community. Thank you!

微信图片_20250202125022.png

Sign up or log in to comment