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 Candle ML framework example #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dimakuv
Copy link

@dimakuv dimakuv commented Jul 26, 2024

Candle is a minimalist ML framework for Rust with a focus on performance and ease of use. This commit adds two examples with Candle: simple matrix multiplication (to quickly test functionality) and Quantized LLaMA (to test performance).

Originally was submitted in core Gramine repo, but it was agreed to move it in this separate repo: gramineproject/gramine#1938


This change is Reviewable

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


-- commits line 6 at r1:
I think we can simply remove this as it's not in the CI-examples any more

Code quote:

(to quickly test functionality)

-- commits line 7 at r1:
ditto

Code quote:

(to test performance)

candle/README.md line 4 at r1 (raw file):

Candle is a minimalist ML framework for Rust with a focus on performance
(including GPU support) and ease of use: https://github.com/huggingface/candle

Can we use inline links? Besides, missing period at the end of this sentence.

Code quote:

https://github.com/huggingface/candle

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


candle/candle_matmul.manifest.template line 3 at r1 (raw file):

# Copyright (C) 2024 Gramine contributors
# SPDX-License-Identifier: BSD-3-Clause

we don't have loader.entrypoint meaning that we have to hold this PR until after the next Gramine release is done

Copy link
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 6 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @kailun-qin)


-- commits line 6 at r1:

Previously, kailun-qin (Kailun Qin) wrote…

I think we can simply remove this as it's not in the CI-examples any more

Done.


-- commits line 7 at r1:

Previously, kailun-qin (Kailun Qin) wrote…

ditto

Done.


candle/README.md line 4 at r1 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Can we use inline links? Besides, missing period at the end of this sentence.

Done.


candle/candle_matmul.manifest.template line 3 at r1 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

we don't have loader.entrypoint meaning that we have to hold this PR until after the next Gramine release is done

Done.

No, I think we should add the loader.entrypoint now, and then apply #99 to all examples, including Candle.

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


candle/README.md line 4 at r2 (raw file):

[Candle](https://github.com/huggingface/candle) is a minimalist ML framework for
Rust with a focus on performance (including GPU support) and ease of use. 

trailing white space?

Code quote:

·

Candle is a minimalist ML framework for Rust with a focus on performance
and ease of use. This commit adds the Quantized LLaMA example.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
Copy link
Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @kailun-qin)


candle/README.md line 4 at r2 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

trailing white space?

Done.

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

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.

2 participants