Skip to content

Conversation

@Man2Dev
Copy link
Contributor

@Man2Dev Man2Dev commented Apr 20, 2024

Added Fedoras dependencies for OpenCL, and CLBlast hardware acceleration

Pre-built CLBlast binaries may be found on the [CLBlast Releases](https://github.com/CNugteren/CLBlast/releases) page. For Unix variants, it may also be found in your operating system's packages.
Linux packaging:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think we want to detail all the install command for all distro.

Copy link
Collaborator

@phymbert phymbert left a comment

Choose a reason for hiding this comment

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

No need to detail how to install clblast

@Man2Dev
Copy link
Contributor Author

Man2Dev commented Apr 20, 2024

No need to detail how to install clblast

the instruction only states how to build it but not that it's avalibale in the native package manager. which can create pointless work if some one is uninformed

@phymbert
Copy link
Collaborator

I just worry the README.md on the welcome page will become difficult to read if we add so much detailled instruction

@Man2Dev
Copy link
Contributor Author

Man2Dev commented Apr 20, 2024

I just worry the README.md on the welcome page will become difficult to read if we add so much detailled instruction

I understand your predicament however, not accepting the patch won't fix the problem the readme is already riddled it inconsistencies and has no sense of cohesion which makes it arguably more un readable.
The way to actually address this issue is by firstly re formatting the readme to make it more cohesive and much easier to read. And having gidelines for formatting which preferbly be inforced by CI.
However, if you are still against merging the pr then how about I write all the fedora instructions in different markdown file and just have the readme link to that? (But then some one has to port the windows, ubuntu ... to there own file at some point as to make it more coherent) in my opinion it would be best to just merge it at this point untill some gidelines have been put in place for documentation.

Copy link
Collaborator

@phymbert phymbert left a comment

Choose a reason for hiding this comment

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

@phymbert phymbert dismissed their stale review April 21, 2024 00:25

no strong opinion

Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

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

The README can be improved, but lets merge for now

@ggerganov ggerganov merged commit 2cca09d into ggml-org:master Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants