5 ms·
Can someone in the know describe what the hullaballoo is about? Seems like ego-driven optimization that breaks compatibility?
by ukuina 4y ago
Can someone in the know describe what the hullaballoo is about?
Seems like ego-driven optimization that breaks compatibility?
- femboy 4y agoWhat's ego-driven optimization?
- oh_sigh 4y agoPushing optimizations for personal glory that look good with some benchmarks but may have unthought of or hidden regressions on other aspects of the code/user base. On the same hand this may be an ego driven revert. from reading the bug it seems like some people might be salty that jart gets a lot of publicity for a few changes where other people which bigger contributions to the project don't get.
- simion314 4y agoThe issue was with the model versioning compatibility change and someone inserting his initials in the magic number. The optimizations worked great for me on Linux, the first tiem your run the CLI program you have to wait a lot of time, more then 1 minute for a 20Gb model , but the next runs are instant. Maybe some Windows guys are salty that this does not work as great on their OS , if that is the case they should update it to get the performance boost. This are not soem nanoseconds you gain but actual minutes for each run.
- trsohmers 4y agoThe tl;dr as I understand it is that jart had a misunderstanding of how what was actually happening and the benefits of the map optimization… the claims of actually being able to shrink the model size from 20GB > 6GB were just completely false, and while there was a model loading time improvement, actual memory required and used did not change. A number of people saw this and said that making a breaking change to the repo that a lot of people are using and have forked for other models was a bad idea, thus this new PR.
- blibble 4y agohere mmap is being used for essentially lazy loading that's it
- chpatrick 4y agoBut then it's pretty much the same as using the old version and turning on swap, so I don't really see the point. As far as I understand the whole model needs to be read constantly so there's no benefit from the random access mmap provides.
- sitkack 4y agoSwap is a system level property not a program level property. They are similar, use similar mechanisms, but the experience that a user would see are very different.
- chpatrick 4y agoI'm not sure it would be that different but mmap has the benefit that it can swap directly to the model file on the disk instead of making a copy in swap space.
- sitkack 4y agommap is like tightly scoped, targeted swap. If the PoR is on disk, the OS is free to reclaim that memory for something else at anytime. It really is a beautiful hack, but if you turn on swap for the system in that way, it has to balance memory usage across all the running programs. In this use of mmap, there is nothing to swap out as the source of truth is always on disk. During general swap usage, memory has to travel in both directions.
- simonw 4y agoThe 20GB to 6GB confusion appears to have come from the title of the Hacker News post the other day: https://news.ycombinator.com/item?id=35393284 https://news.ycombinator.com/item?id=35393284 The PR it linked to said nothing of the sort: https://github.com/ggerganov/llama.cpp/pull/613 https://github.com/ggerganov/llama.cpp/pull/613
- augusto-moura 4y agoIs not ego-driven, the requirements for running any model dropped by more than half, you can know run the largest model, 30b, on domestic over the shelf computers. The change is very welcome in the community
- tempaccount420 4y agoThere were no improvements to memory use, the earlier GBs were a measurement error. If you couldn't run a model before (or were swapping, so running very slow), then you still have the same problem. You will actually "swap" a lot more than before if you have barely enough memory, but this is fixable with the --mlock flag. Edit: For everyone downvoting, please tell me what is wrong with my comment. I don't have a bone in this fight.
- xiphias2 4y ago4chan came here to chime in a bit. You were downvoted by 10 year old children, good luck explaining to them how memory locking works :)
- klohto 4y agommap breaks previous (not guaranteed) compatibility and (few) people demand option to turn it off by reverting all of the commits and throwing hands up by demanding more testing, documenting, and more optional arguments. While I agree with some of the premises, it’s up to ggerganov in the end. If he wants this to be the default, so be it. Throwing a (polite) tantrum because you should please all, while not offering to do any of the work, is entitled demand.
- 4bpp 4y agoA project that has been generating a lot of buzz lately (CPU-based inference for Facebook's LLaMa model that works on commodity hardware) has attracted contributions from a tech/activist celebrity (https://en.wikipedia.org/wiki/Justine_Tunney https://en.wikipedia.org/wiki/Justine_Tunney). Their somewhat overly self-assured/-aggrandizing style (e.g. Github posts written in a tone like they run the place, changing the file format magic number to include their own initials along those of the project's originator) rubbed many people the wrong way, and a sweeping change they introduced may have resulted in performance regressions for several users (while also being hugely oversold: a fantastical and quickly disproven claim about significantly reduced memory usage sat at >1k upvotes on HN yesterday). Then another long-standing contributor made a PR just seeking to flat out revert the patch in question. The discussion quickly turned toxic, with a (thankfully) not-yet-quite-vocalised US culture war undercurrent and people bandwagoning based on their personal disposition towards the person at the core of it.
- fastball 4y agoAlso worth mentioning that there is some level of controversy over how much of this work involving mmap should be attributed to jart vs slaren. Slaren originally authored a PR using mmap which some people are claiming was the much better implementation of the feature (including not needing to change the model format) and jart basically re-wrote it so that she could take credit for it.
- detaro 4y ago> basically re-wrote it so that she could take credit for it. compare the actual PR: https://github.com/ggerganov/llama.cpp/pull/613 https://github.com/ggerganov/llama.cpp/pull/613 You'll note that it includes the original commits from Slaren, explicitly mentions the collaboration with Slaren and explicitly requests to preserve these commits on merge.