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

Arkworks conversion example #619

Merged
merged 24 commits into from
Sep 30, 2024
Merged

Arkworks conversion example #619

merged 24 commits into from
Sep 30, 2024

Conversation

yshekel
Copy link
Collaborator

@yshekel yshekel commented Sep 26, 2024

This PR:
(1) introduces a new Rust example for converting Arkworks types to ICICLE and back. It is meant to be used as reference for users using ICICLE.
(2) update to/from Montgomery trait and implementations to support HostOrDeviceSlice instead of DeviceSlice only so that it can work for host data as well.
(3) choose better c for cpu msm for way faster cpu (for non precompute case)

@yshekel yshekel force-pushed the yshekel/arkworks_example branch from ca75a7f to dfff31b Compare September 29, 2024 11:50
@yshekel yshekel force-pushed the yshekel/arkworks_example branch from a734c41 to de9219e Compare September 29, 2024 15:17
@yshekel yshekel marked this pull request as ready for review September 29, 2024 15:35
@yshekel yshekel requested a review from vhnatyk September 30, 2024 09:12
@yshekel yshekel requested a review from vhnatyk September 30, 2024 11:49
examples/rust/arkworks/src/main.rs Outdated Show resolved Hide resolved
examples/rust/arkworks/src/main.rs Outdated Show resolved Hide resolved
I: FieldImpl + MontgomeryConvertible,
{
// SAFETY: Reinterpreting Arkworks field elements as Icicle-specific scalars
let icicle_scalars = unsafe { &*(&ark_scalars[..] as *const _ as *const [I]) };
Copy link
Contributor

Choose a reason for hiding this comment

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

Rust recommended to remove the double slicing here, since ark_scalars is already a slice. I think it allocates memory again when creating this extra slice. Minor change.

fn ark_to_icicle_scalars_async<T, I>(ark_scalars: &[T], stream: &IcicleStream) -> DeviceVec
where
T: PrimeField,
I: FieldImpl + MontgomeryConvertible,
{
// SAFETY: Reinterpreting Arkworks field elements as Icicle-specific scalars
let icicle_scalars = unsafe { &*(ark_scalars as *const _ as *const [I]) };

// Create a HostSlice from the mutable slice
let icicle_host_slice = HostSlice::from_slice(icicle_scalars);

let mut icicle_scalars = DeviceVec::<I>::device_malloc_async(ark_scalars.len(), stream).unwrap();
icicle_scalars
    .copy_from_host(icicle_host_slice)
    .unwrap();

// Convert from Montgomery representation using the Icicle type's conversion method
I::from_mont(&mut icicle_scalars, stream);
icicle_scalars

}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated but it doesn't seem to be faster

@yshekel yshekel requested a review from krakhit September 30, 2024 13:23
@yshekel yshekel merged commit 1da4536 into main Sep 30, 2024
28 checks passed
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