Skip to content

Conversation

ngxson
Copy link
Collaborator

@ngxson ngxson commented Jun 5, 2025

Maybe too late to fix upstream model (one of the GGUF already set to public), so I think we can do a quick patch in llama.cpp

CC @yuhao318 do you have any visibility about why only one GGUF is public?

@ngxson ngxson requested a review from ggerganov June 5, 2025 11:48
Comment on lines 822 to +823
ml.get_key(LLM_KV_ATTENTION_LAYERNORM_RMS_EPS, hparams.f_norm_rms_eps);
hparams.pooling_type = LLAMA_POOLING_TYPE_LAST; // for embeddings model
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ml.get_key(LLM_KV_ATTENTION_LAYERNORM_RMS_EPS, hparams.f_norm_rms_eps);
hparams.pooling_type = LLAMA_POOLING_TYPE_LAST; // for embeddings model
hparams.pooling_type = LLAMA_POOLING_TYPE_LAST; // for embeddings model
ml.get_key(LLM_KV_POOLING_TYPE, hparams.pooling_type, false);
ml.get_key(LLM_KV_ATTENTION_LAYERNORM_RMS_EPS, hparams.f_norm_rms_eps);

Copy link
Collaborator

@CISC CISC Jun 5, 2025

Choose a reason for hiding this comment

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

Perhaps use metadata if it exists, and add the pooling check to conversion (do they use 1_Pooling)? Ah, the check is already there, guess they don't.

Copy link
Collaborator Author

@ngxson ngxson Jun 5, 2025

Choose a reason for hiding this comment

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

For rerank model, there is be a KV metadata to switch the pooling to "rank"

So yes, it's better to do as you suggested

@ngxson
Copy link
Collaborator Author

ngxson commented Jun 5, 2025

superseded by the other PR

@ngxson ngxson closed this Jun 5, 2025
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