imatrix : use GGUF to store importance matrices #9400
Conversation
* perplexity : simplify filling the batch
Sums and counts tensors no longer need to be consecutive. * imatrix : more sanity checks when loading multiple imatrix files * imatrix : use ggml_format_name instead of std::string concatenation Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
|
I'm setting this to "draft", because of concerns by @ikawrakow in ikawrakow/ik_llama.cpp#15 (comment) and ikawrakow/ik_llama.cpp#15 (comment) (mostly related to the fact that GGUF is harder to parse than More details near the end of ikawrakow/ik_llama.cpp#15 (reply in thread). I'll need some days to think about how to go further with this. |
|
@compilade This is a good change and I think it would be useful to bring it to a completion. In the future, we can extend |
This should make comparisons between the formats easier because this matches the behavior of the previous version.
tools/imatrix/imatrix.cpp
Outdated
| static bool str_remove_suffix(std::string & str, const std::string & suffix) { | ||
| bool has_suffix = str_has_suffix(str, suffix); | ||
| if (has_suffix) { | ||
| str = str.substr(0, str.size() - suffix.size()); | ||
| } | ||
| return has_suffix; | ||
| } |
There was a problem hiding this comment.
| static bool str_remove_suffix(std::string & str, const std::string & suffix) { | |
| bool has_suffix = str_has_suffix(str, suffix); | |
| if (has_suffix) { | |
| str = str.substr(0, str.size() - suffix.size()); | |
| } | |
| return has_suffix; | |
| } | |
| static bool str_remove_suffix(std::string & str, const std::string_view & suffix) { | |
| bool has_suffix = string_ends_with(str, suffix); | |
| if (has_suffix) { | |
| str = str.substr(0, str.size() - suffix.size()); | |
| } | |
| return has_suffix; | |
| } |
This is a nice complement to string_ends_with, should be moved to common.cpp as string_remove_suffix.
There was a problem hiding this comment.
Shouldn't std::string_view be passed by value instead of by const reference in these functions?
For example, https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/ suggests that std::string_view should always be passed by value.
There was a problem hiding this comment.
I did wonder about that, but didn't know enough of the details behind it, reading that it certainly seems so, and it should be worth going over the usage of std::string_view elsewhere.
Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@scala.com>
There was a problem hiding this comment.
LGTM, I did some light testing with old and new formats, converting them back and forth, checking that quantization was unchanged, etc, all seems good.
Only slightly annoying thing is the garbage output to console (by Resolved in #14381gguf_init_from_file) when using old format.
Will probably benefit from testing by more people before merging, @bartowski1182 @danielhanchen ?
|
Thank you for working on this, I've been thinking that storing imatrix as gguf wold be nice for investigating the use of gradients instead of activations. |
Follow-up from ikawrakow/ik_llama.cpp#15 (reply in thread).
Using GGUF as the format for
imatrixfiles will be useful for further experiments (e.g. with L²QER) and compatibility with existing or future GGUF tooling (e.g. GGUF previews on HuggingFace, graphical GGUF viewer(s) #6715, some kind ofgguf-diff, etc.).There are multiple problems with
imatrixwhich this is addressing:unordered_mapiteration order (makessha256sumuseless to compareimatrixfiles made on the same dataset)-ub(intermediate saves happen waaay too often)Summary of changes
imatrixdata.general.typeisimatrixgeneral.architectureimatrixfiles.*.in_sum2and*.countsfor each tensors with imatrix data.*.in_sum2are the per-channel sums of squared activationsF32, like before.*.countsare the number of activations (also the number of tokens), useful to calculate the mean squared activations (which is used byllama-quantize)imatrixfiles together with--in-file.F32even though it's integer values, because when calculating the mean it would be converted toF32anyway to perform the division.Addconvert_legacy_imatrix_to_gguf.pyto convert oldimatrix.datfiles toimatrix.ggufllama-quantizecan still read the old format (with a warning)) or can be converted withllama-imatrixdirectly (when the output file has the.ggufsuffix).llama-perplexitysince perplexity : support using multiple sequences to allow larger batch sizes #5946, allow computing multiple chunks per batch withllama-imatrixUse fused-multiply-add (withstd::fma) when accumulating the sums of activationsllama-imatrixfrommaster)f64would be even better, but I'm not use it's worth it yet. For the curious, usingdoublefor the intermediate accumulations can be tried by changing only one line inIMatrixStats:vector<float> valuestovector<double> values.)unordered_map.sha256sumcan be meaningfully used to compareimatrixfiles generated in very similar conditions.TODO
llama-quantizewith oldimatrix.datwith newllama-quantizeusing convertedimatrix.ggufsha256sum.llama-imatrixat different batch sizes-ub 64 -b 512and-ub 512 -b 2048for a chunk size of 512 (-c 512)llama-imatrixvs newllama-imatrix--in-filewithllama-imatrix.imatrixor.ggufimatrix (for round-trip conversions)general.architectureexclusion.self.add_architecture()a no-op, but maybegeneral.architectureshould simply be excluded whenself.arch == "". Not sure how to prevent using the otherself.add_*(inGGUFWriter) which expectself.archto be something.imatrix.datfiles?