Skip to content

Conversation

DamonFool
Copy link
Contributor

Since data = data_torch.numpy() has been called at line 303, line 307 seems to be redundant.
Or am I missing something?
Thanks.

@github-actions github-actions bot added the python python script changes label Sep 1, 2025
Comment on lines -305 to -308
# if data ends up empty, it means data_torch was a scalar tensor -> restore
if len(data.shape) == 0:
data = data_torch.numpy()

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the intention here was to have the shape be non-zero for scalars, in which case

data = data.reshape(1)

might be more appropriate in that otherwise redundant if condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @compilade for the comments.

Is there a case that data.reshape(1) is needed when converting gguf models?
If not, there might be some risk adding data.reshape(1) here.

I would suggest removing the code at the moment which doesn't change the logic.
And we can still add data.reshape(1) in the future if it really matters.

What do you think?
Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a case that data.reshape(1) is needed when converting gguf models?

Yes, and it's exactly when len(data.shape) == 0 (aka when the shape is an empty tuple ()).

It doesn't really happen in practice because most models don't have scalar tensors.

I would suggest removing the code at the moment which doesn't change the logic.
And we can still add data.reshape(1) in the future if it really matters.

Agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @ggerganov and @compilade for your approval.

@taronaeo taronaeo merged commit 4b20d8b into ggml-org:master Sep 1, 2025
7 checks passed
@DamonFool
Copy link
Contributor Author

Thanks @taronaeo .

@DamonFool DamonFool deleted the convert branch September 1, 2025 23:09
walidbr pushed a commit to walidbr/llama.cpp that referenced this pull request Sep 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python python script changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants