Skip to content

Conversation

@yandai
Copy link

@yandai yandai commented Jul 8, 2023

Hi,

In current llama implement, post_decoder_embedding.bias seems uninitialized then cause crash .
Try to fix this problem and please help to take a look.

Thanks

@void-main
Copy link
Owner

Hi @yandai , thanks for the contribution, may I ask when we need post_decoder_embedding.bias? Which model are you using?

@yandai
Copy link
Author

yandai commented Jul 10, 2023

As my understanding,llama don't use bias. So gpt_weights->post_decoder_embedding.bias and padded_embedding_bias_ are not initialized correctly. My change is also compatible with "llama with post_decoder_embedding.bias".

ref:
https://github.com/huggingface/transformers/blob/main/src/transformers/models/llama/modeling_llama.py#L620

@void-main
Copy link
Owner

@yandai , I agree that llama doesn't use bias, that's why I wonder why we need to add that if/else test here?

@yandai
Copy link
Author

yandai commented Jul 13, 2023

So there's no need to keep padded_embedding_bias_ so I think removing the padded_embedding_bias_ member is better choice.

@void-main
Copy link
Owner

So there's no need to keep padded_embedding_bias_ so I think removing the padded_embedding_bias_ member is better choice.

Hi @yandai , agree! So, do you think it's a good idea that you change your commit by simply removing lines 823-827 from Llama.cc, since we don't care bias anyways.

@yandai
Copy link
Author

yandai commented Jul 16, 2023

okay, padded_embedding_bias_ is removed and please take a look.

@void-main void-main merged commit e770ddf into void-main:main Jul 17, 2023
@void-main
Copy link
Owner

merged, thanks!

@ryxli
Copy link

ryxli commented Jul 20, 2023

Should also remove post_decoder_embedding.bias = nullptr added in LlamaWeight.cc?

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.

3 participants