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

Update icicle version #18

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Update icicle version #18

wants to merge 7 commits into from

Conversation

nonam3e
Copy link

@nonam3e nonam3e commented Mar 29, 2024

No description provided.

var newElement icicle_bn254.Projective
FromG1AffineGnark(&e, &newElement)

newElements = append(newElements, *StripZ(&newElement))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should use newElement.ProjectiveToAffine. I think it is more future proof for cases where z != 1

Comment on lines +15 to 17
fb := f.ToBytesLittleEndian()
var b32 [32]byte
copy(b32[:], fb[:32])
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should use fp.Bytes here instead of hardcoded values for the size of the array

type OnDeviceData struct {
P unsafe.Pointer
Size int
func MsmOnDevice(gnarkPoints []bn254.G1Affine, gnarkScalars []fr.Element) (*bn254.G1Affine, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the previous API, MsmOnDevice was meant to use data already on the device. I suggest we either make a generic MSM function that can accept both DeviceSlice and Host data OR have two separate functions each handling one case

iciclePoints := HostSliceFromPoints(gnarkPoints)
icicleScalars := HostSliceFromScalars(gnarkScalars)

cfg := core.GetDefaultMSMConfig()
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should expose this in the API so users can use streams and change the primitive config depending on their own needs (like setting that points and/or scalars are in Montgomery form).

cfg := core.GetDefaultMSMConfig()
var p icicle_bn254.Projective
var out core.DeviceSlice
_, e := out.Malloc(p.Size(), p.Size())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This currently only supports a single MSM and not batching. We should update the API to allow for batched MSM operations

}

func INttOnDevice(scalars_d, twiddles_d, cosetPowers_d unsafe.Pointer, size, sizeBytes int, isCoset bool) unsafe.Pointer {
ReverseScalars(scalars_d, size)
func Ntt[T any](gnarkScalars fr.Vector, dir core.NTTDir, cfg *core.NTTConfig[T]) (fr.Vector, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't support using data already on the device or keeping results on the device for later use

Comment on lines +58 to +70
// func INttOnDevice(scalars_d, twiddles_d, cosetPowers_d unsafe.Pointer, size, sizeBytes int, isCoset bool) unsafe.Pointer {
// ReverseScalars(scalars_d, size)

ReverseScalars(scalars_out, size)
}
// scalarsInterp := icicle_bn254.Interpolate(scalars_d, twiddles_d, cosetPowers_d, size, isCoset)

func MsmOnDevice(scalars_d, points_d unsafe.Pointer, count int, convert bool) (bn254.G1Jac, unsafe.Pointer, error) {
pointBytes := fp.Bytes * 3 // 3 Elements because of 3 coordinates
out_d, _ := goicicle.CudaMalloc(pointBytes)
// return scalarsInterp
// }

icicle.Commit(out_d, scalars_d, points_d, count, 10)
// func NttOnDevice(scalars_out, scalars_d, twiddles_d, coset_powers_d unsafe.Pointer, size, twid_size, size_bytes int, isCoset bool) {
// res := icicle_bn254.Ntt(scalars_out, scalars_d, twiddles_d, coset_powers_d, size, twid_size, isCoset)

if convert {
outHost := make([]icicle.G1ProjectivePoint, 1)
goicicle.CudaMemCpyDtoH[icicle.G1ProjectivePoint](outHost, out_d, pointBytes)
// if res.IcicleErrorCode != core.IcicleErrorCode(0) {
// fmt.Print("Issue evaluating")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove any commented code

func INttOnDevice(scalars_d, twiddles_d, cosetPowers_d unsafe.Pointer, size, sizeBytes int, isCoset bool) unsafe.Pointer {
ReverseScalars(scalars_d, size)
func Ntt[T any](gnarkScalars fr.Vector, dir core.NTTDir, cfg *core.NTTConfig[T]) (fr.Vector, error) {
icicleScalars := core.HostSliceFromElements(BatchConvertFromFrGnark(gnarkScalars))
Copy link
Collaborator

Choose a reason for hiding this comment

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

For ntt, we really only need to convert from uint64 to uint32 since Montgomery form is supported

@nonam3e nonam3e changed the title Update icicle version Update golang bindings to icicle v2 Apr 10, 2024
@nonam3e nonam3e changed the title Update golang bindings to icicle v2 Update icicle version Apr 10, 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.

2 participants