Skip to content

fix: fall back to a forward pass when generate returns no hidden states - #513

Open
anthony-maio wants to merge 1 commit into
p-e-w:masterfrom
anthony-maio:pr-hidden-states
Open

anthony-maio wants to merge 1 commit into
p-e-w:masterfrom
anthony-maio:pr-hidden-states

Conversation

@anthony-maio

Copy link
Copy Markdown

Hello Phillip, I'm Anthony. I had to make a few changes while I was abliterating a new model yesterday (ZGCM-1-7B its on hf) and this PR is the result. This is not my agent, feel free to reach out with questions but I have to admit I don't check in too often (I contribute to a lot also) so I might be a minute.

ZGCM-1-7B is a model that punches above its weight and i thought it would be fun to use with reduced guardrails, and this model is a bit odd because it drops the outout_hidden_states flag through it's generate call which sends None in and causes the whole thing to blow up due to hidden_states coming in as None. I'm trying to make it clear I'm a human writing this, happy to dig in deeper if you want to chat but I feel like it's kind of obvious once you take a look at this model code on hf, or just explore the scneario I described. I have a few more to send over, but writing these is a chore, they're on my fork, few more fixes for this model. ZGCM-1-7B.

AI use declaration

Check either of the following:

  • I wrote the code in this PR entirely by myself.
  • I used AI to write some or all of the code in this PR, and I have reviewed the entire code myself
    and fully understand it. I'm prepared to discuss the code without letting an agent speak on my behalf.

Comment thread src/heretic/model.py Outdated

if hidden_states is None:
# Some remote-code models (e.g. ZGCM-1) rebuild the model inputs in their own
# prepare_inputs_for_generation and drop output_hidden_states along the way,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why do they do this? I assume they do some kind of input preprocessing? If so, wouldn't this logic destroy that?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

honestly it looks like the intended inference path for this model was vllm. i never really considered why they did it at first because I was just trying to abliterate a novel model, but if you look at the modeling file, they basically reimplemented all of the standard interfaces when using hf transformers and in doing so, they broke 3 different interface contracts, this being one of them. my change didn't break this because they didn't implement it properly. upon further consideration, this change is too much of a one off and although the initial reasoning for submitting it was that surely this is something that could be seen elsewhere, my change would potentially negative impact other models.... although for this case and most I can conceive of it's a no-op. I pushed a more conservative fix, but overall, I am doubting the value of this change compared side-effects, although I believe this last push is more conservative. I would close it, but you already spent time on it, and I'll leave it to you. It won't hurt my feelings either way and I appreciate you taking the time to dig in here. best!

get_residuals reads the residual stream from
generate(..., output_hidden_states=True). A model whose remote code rebuilds
the inputs in its own prepare_inputs_for_generation, returning a fresh dict
rather than merging the remaining kwargs, drops that flag on the way through.
generate then returns hidden_states = (None,) and abliteration dies with
"TypeError: 'NoneType' object is not iterable" before the first trial.

ZGCM-1-7B does this: its prepare_inputs_for_generation returns only
input_ids, attention_mask, position_ids, past_key_values and use_cache.

When that happens, run one forward pass over the same tokenized inputs with
output_hidden_states=True. The hidden states at the last prompt position are
the ones generate would have reported, so the directions are unchanged. The
fallback supplies left-padding-aware position ids itself, since a model that
would otherwise default to a plain arange over the padded sequence places the
real tokens at the wrong absolute positions. That is a no-op under purely
relative (RoPE) position encoding, where shifting a contiguous block of
tokens cancels out, and necessary for absolute encodings.

Costs one extra forward pass on the affected models and nothing on the rest.
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.

2 participants