Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sync : ggml (backend v2) #3912
sync : ggml (backend v2) #3912
Changes from 7 commits
aa7a2c4
e819070
4fe646f
83c96d5
8401e3e
815f44e
16e819d
e2349ec
f3fb45b
7f8e2a5
075ee61
dc22db7
b1592ea
e50ab5a
081a86d
aa1f36c
a4de804
548ec46
9efc6b9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slaren Does this look OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, I don't know if
gb
here needs grads or not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gb
needs grads, becausegb
also contains thegf
nodes, which have grads.Changing the
bool grads
argument fromfalse
totrue
resolves a triggered assert in ggml.cggml_graph_cpy
.With this change finetune runs, I will report back if the results are good as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should
ggml_graph_cpy
be changed to allow skipping the grads if the src has them but not the dst?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an additional - unrelated to this PR - regression in finetune and train-text-from-scratch due to new yarn rope implementation.
Changing
bool grads
argument to true and applying #3974 to fix the backward process of rope, the output of finetune is correct.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to note, lines 1805 and 1807 below needs that change as well, I missed them at first attempt to copy this fix.
Also, mentioned regression and #3974 fix seems to be critical, because otherwise finetune produces LORA's without any progress from one checkpoint to another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
gb_tmp
also needs thegrads=true
argument.