Skip to content

Set return_dict=True and remove output_hidden_states in Transformer #7

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.

Sign up for GitHub

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

Open
arthrod wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Set return_dict=True and remove output_hidden_states in Transformer #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
10ec27c
Update encoder.py
mayank-rakesh-mck Feb 13, 2025
99f8c9a
Update custom_train.py
mayank-rakesh-mck Feb 13, 2025
2d973ce
Update custom_train.py
mayank-rakesh-mck Feb 13, 2025
a81645d
Update custom_train.py
mayank-rakesh-mck Feb 13, 2025
4c9d474
Update custom_train.py
mayank-rakesh-mck Feb 13, 2025
1f00c9f
Update custom_train.py
mayank-rakesh-mck Feb 14, 2025
d5e9c2f
Update custom_train.py
mayank-rakesh-mck Feb 14, 2025
928787b
Update base.py
mayank-rakesh-mck Feb 24, 2025
023bf75
Update base.py
mayank-rakesh-mck Feb 24, 2025
e017e04
Update base.py
mayank-rakesh-mck Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion custom_train.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,4 @@ def create_parser():
trainer = Trainer(config, allow_distributed=args.allow_distributed,
compile_model = args.compile_model,
device='cuda' if torch.cuda.is_available() else 'cpu')
trainer.run()
trainer.run()
4 changes: 2 additions & 2 deletions gliner/modeling/encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def forward(self, *args, **kwargs):
output_hidden_states = True
else:
output_hidden_states = False
output = self.model(*args, output_hidden_states = output_hidden_states,
output = self.model(*args, #output_hidden_states = output_hidden_states,

@gemini-code-assist gemini-code-assist Bot Feb 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change introduces a potential bug. By commenting out output_hidden_states, the model will rely on its default configuration. If self.config.fuse_layers is True (checked on line 96), line 103 accesses output.hidden_states, which will likely not be present in the model's output, causing an AttributeError at runtime. The output_hidden_states parameter must be explicitly set to True when layer fusion is enabled.

Instead of commenting out this line, the logic should be corrected and simplified. The entire block from line 96 to 101 can be replaced with:

        output = self.model(
            *args,
            output_hidden_states=self.config.fuse_layers,
            return_dict=True,
            **kwargs
        )

This is more concise and ensures output_hidden_states is correctly passed when needed.

return_dict = True, **kwargs)
Comment on lines +100 to 101

@sourcery-ai sourcery-ai Bot Feb 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Passing output_hidden_states is commented out but related logic is still active, which may break fuse_layers behavior.

Since output_hidden_states is still derived from self.config.fuse_layers but no longer passed to self.model, output.hidden_states may be absent if the model’s default is output_hidden_states=False, breaking or degrading self.layers_fuser. Either remove the now-dead output_hidden_states logic and explicitly handle the fuse_layers path, or restore/gate the argument instead of commenting it out in place.

Comment on lines +100 to 101

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pass hidden states when layer fusion is enabled

When fuse_layers is enabled, this forward path still computes output_hidden_states = True but no longer forwards it to the transformer call, so output.hidden_states is typically None for standard HF configs and self.layers_fuser(output.hidden_states) will fail at runtime (or produce invalid fusion input) for any run using layer fusion. This change effectively breaks the fuse_layers feature introduced in this module.

Useful? React with 👍 / 👎.

if self.config.fuse_layers:
encoder_layer = self.layers_fuser(output.hidden_states)
Expand Down Expand Up @@ -165,4 +165,4 @@ def forward(self, input_ids, attention_mask,
token_embeddings = self.encode_text(input_ids, attention_mask, *args, **kwargs)

labels_embeddings = self.encode_labels(labels_input_ids, labels_attention_mask, *args, **kwargs)
return token_embeddings, labels_embeddings
return token_embeddings, labels_embeddings