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:
- It seems that
self.img_context_token_id
is used in theforward
method, but it’s not initialized within that method. - The
forward
method is the only one that directly returns the attention output. - 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:
- If
forward
is called directly, it may fail due to uninitialized variables. - Tasks that depend on the attention output might be affected if this initialization is missed.
Possible Solutions:
- Would it be beneficial to initialize
self.img_context_token_id
within the__init__
method to ensure it’s always defined? - Or should we add initialization logic to the
forward
method itself, similar to the other methods? - 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!