Skip to content
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

Add Rosetta2 Binaries #755

Merged
merged 5 commits into from
Jun 2, 2024
Merged

Conversation

abhiaagarwal
Copy link
Contributor

Closes #752. I make the assumption that "if x86 and doesn't have AVX, then it is rosetta2". My understanding is that all modern (>2015)-based Intel CPUs used on Macbooks have AVX, so checking for AVX under a x86 environment is a good enough proxy for "under Rosetta2".

@abhiaagarwal
Copy link
Contributor Author

@SignalRT
Copy link
Collaborator

https:/abhiaagarwal/LLamaSharp/actions/runs/9237231679/job/25413958437 Artifacts

@abhiaagarwal, it seems that the step to Gather binaries fail in linked execution. I would appreciate If you can check this.

The other thing that seems strange to me is to name this as something related with rosetta. Any intel osx without avx support is included in this use case (including rosetta emulation). Am I right or I'm missing something?.

@abhiaagarwal
Copy link
Contributor Author

I'll fix the issue! Missing adding the folder name in mkdir.

The other thing that seems strange to me is to name this as something related with rosetta. Any intel osx without avx support is included in this use case (including rosetta emulation). Am I right or I'm missing something?.

Yes, but at least in regards to Macbooks on Intel, non-AVX hardware doesn't really exist anymore. I did a search and the last chip in an Intel Macbook was in 2014. I named it Rosetta2 to make it more clear to anyone poking around why these binaries exist, but I don't really have a strong opinion otherwise. More than happy to rename it to nonavx if desired.

@martindevans
Copy link
Member

@abhiaagarwal Could you start another test run on your repo please. If that works we can merge this :)

@abhiaagarwal
Copy link
Contributor Author

@martindevans absolutely! Fixed the mkdir issue and triggered a new run here. https:/abhiaagarwal/LLamaSharp/actions/runs/9294309458

@abhiaagarwal
Copy link
Contributor Author

Seems like the hardest thing I've ever done is properly creating directories... here's a run that succeeded. https:/abhiaagarwal/LLamaSharp/actions/runs/9294794909

@martindevans
Copy link
Member

martindevans commented May 30, 2024

Looks good to me! @SignalRT I think this is ready to merge if it looks ok to you?

Nite: CI is failing due to the missing binaries. Once we merge this I can immediately start a new build run, include the binaries in #751, and we can merge it right away.

@AsakusaRinne
Copy link
Collaborator

There's one more thing: could you please add the binaries to this nuspec file, so that it will get published in our next release?

Besides, @martindevans @SignalRT do you think it's a good idea to separate the MAC backend package from LLamaSharp.Backend.Cpu? Actually MAC binaries are not "pure" CPU ones because they may take use of metal.

@martindevans
Copy link
Member

do you think it's a good idea to separate the MAC backend package from LLamaSharp.Backend.Cpu?

Seems like a good idea to me, it's a good way to shrink the CPU backend package down a bit.

@abhiaagarwal
Copy link
Contributor Author

If you can open a new branch on this repo, I can change this PR to merge against that? I'm not familiar with dotnet packaging, but a mac-specific backend is for sure a good idea.

@SignalRT
Copy link
Collaborator

The PR it's ok for me. I would not separate the backend at this moment. In my opinion the point that rest is to add de libraries to the nuspec

@abhiaagarwal
Copy link
Contributor Author

abhiaagarwal commented May 31, 2024

Updated the nuspec. Do y'all want me to squash my commits?

Copy link
Collaborator

@SignalRT SignalRT left a comment

Choose a reason for hiding this comment

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

It seems OK to me.

@martindevans
Copy link
Member

No need for you to squash, we can squash-and-merge :)

@martindevans martindevans merged commit 2791f20 into SciSharp:master Jun 2, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLamaSharp runtime binaries don't support Rosetta2
4 participants