Skip to content

Delete SHA256SUMS for now #416

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 23, 2023
Merged

Delete SHA256SUMS for now #416

merged 1 commit into from
Mar 23, 2023

Conversation

anzz1
Copy link
Contributor

@anzz1 anzz1 commented Mar 23, 2023

Delete this for now to avoid confusion since it contains some wrong checksums from the old tokenizer format

Re-add after #374 is resolved

@anzz1
Delete SHA256SUMS for now
10526e8 
Delete this for now to avoid confusion since it contains some wrong checksums from the old tokenizer format
Re-add after #374 is resolved
@anzz1 anzz1 added documentation Improvements or additions to documentation high priority Very important issue labels Mar 23, 2023
@anzz1 anzz1 requested a review from rgerganov March 23, 2023 07:41
sw
sw approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@sw sw left a comment

Choose a reason for hiding this comment

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

I agree that it's best not to confuse people with wrong checksums

@sw sw merged commit 8eea5ae into master Mar 23, 2023
@Green-Sky
Copy link
Collaborator

Green-Sky commented Mar 23, 2023 •
edited
Loading

it would have been better to only delete the wrong old checksum 👀

@sw sw deleted the patch-delete-sha256sums branch March 23, 2023 10:40
@anzz1
Copy link
Contributor Author

anzz1 commented Mar 23, 2023

it would have been better to only delete the wrong old checksum eyes

I wasn't sure if all of them were wrong/old or just some. In any case better to take it out for now until it's fully updated and checked first, right?

@Green-Sky
Copy link
Collaborator

afaik most where correct, @gjmulder ?

@gjmulder
Copy link
Collaborator

@Green-Sky they were "correct" in that they reflected the *.pth data files I have and ggml files generated from them at that point in time, with the exception of the alpaca ggml which I found in various places.

I was waiting for people to confirm that everything was correct before merging, but @prusnak decided to go ahead with what I had generated.

@prusnak
Copy link
Collaborator

prusnak commented Mar 23, 2023 •
edited
Loading

This was a mistake. There was no need to erase all hashes, we could have kept original file hashes (PTH).

Please leave more time for review than just 3 hours.

@sw
Copy link
Contributor

sw commented Mar 23, 2023

I apologize for the premature merge. Shall I restore the pth hashes?

sw added a commit that referenced this pull request Mar 23, 2023
@sw
Revert "Delete SHA256SUMS for now (#416)"
e0607ae 
This reverts commit 8eea5ae.
@sw sw mentioned this pull request Mar 23, 2023
Merged
sw added a commit that referenced this pull request Mar 23, 2023
@sw @prusnak
Revert "Delete SHA256SUMS for now" (#429)
a50e39c 
* Revert "Delete SHA256SUMS for now (#416)"

This reverts commit 8eea5ae.

* Remove ggml files until they can be verified
* Remove alpaca json
* Add also model/tokenizer.model to SHA256SUMS + update README

---------

Co-authored-by: Pavol Rusnak <pavol@rusnak.io>
@Bearsaerker Bearsaerker mentioned this pull request Mar 12, 2025
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@sw sw

@blackhole89 blackhole89

@ggerganov ggerganov

@Green-Sky Green-Sky

@rgerganov rgerganov

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation high priority Very important issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@anzz1 @Green-Sky @gjmulder @prusnak @sw