-
Notifications
You must be signed in to change notification settings - Fork 36
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
GPT2 Integrated Gradients - empty input gives false results #190
Comments
Thank you for the detailed bug report @Victordmz, very appreciated!
inseq/inseq/models/huggingface_model.py Lines 271 to 277 in b5d3610
(see #123 for a proposed improvement to enable greater flexibility). In the case of GPT-2, the UNK token corresponds to the EOS token To summarize, action points here would be:
Would you be willing to help with any of these? I cannot commit to these improvements in the upcoming month, but can help out if you're willing to give it a shot! |
Update: the BOS omission logic was removed, and the current behavior in the This: import inseq
model = inseq.load_model("gpt2", "integrated_gradients")
model.attribute(
"",
"This is a demo sentence."
).show() is now equivalent to this: import inseq
model = inseq.load_model("gpt2", "integrated_gradients")
model.attribute(
"<|endoftext|>",
"<|endoftext|> This is a demo sentence."
).show() Closing this, as the choice for alternative baselines beyond the default UNK token (point 3 in the summary) is already document in issue #123. |
🐛 Bug Report
When leaving the input texts empty for GPT2 with integrated gradients, the saliency map seems to be incorrect and giving false results. The goal is to only give <|endoftext|>, the BOS token, as input (and let GPT-2 generate from nothing basically), which can be done by leaving the input empty.
The problem is here:
inseq/inseq/attr/feat/feature_attribution.py
Line 303 in b5d3610
inseq/inseq/models/decoder_only.py
Lines 177 to 182 in b5d3610
The call to
TextSequences
in this method setsskip_special_tokens
to True, removing the<|endoftext|>
from the input. This also prevents a user from giving<|endoftext|>
as the only input (and at the start of the generated text), since it is removed in the input. In that case, when running, there will be an error that the generated text does not begin with the input text.It can be resolved by temporarily changing the line to:
However, the feature attribution is zero for every <|endoftext|> token in the input and the output. I'm not sure whether or not this is meant to be, the same process with the ecco package gives attribution to this token. Also, the first token (in this case This) gets zero attribution, which is probably not supposed to be the case.
Summary:
<|endoftext|>
as input because it is removed when processing.<|endoftext|>
is zero. This is probably not correct.🔬 How To Reproduce
Steps to reproduce the behavior:
Code sample
Environment
Expected behavior
See bug report. This is the integrated gradients result from the ecco package on the same sentence, also using integrated gradients:
I assume this would be correct, however, they leave the baseline default.
The text was updated successfully, but these errors were encountered: