golang v3 wrappers #564
Closed
golang v3 wrappers #564
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Jeremy Felder <[email protected]>
Co-authored-by: Jeremy Felder <[email protected]>
…-zk/icicle into feat/vlad/refactor-from-affine
Fix typos
## Describe the changes This PR refactors the different affine to projective conversion functions using the C function also small bug fix for ProjectiveToAffine() function in Go ## Linked Issues Resolves #
Fix typos
Co-authored-by: Jeremy Felder <[email protected]>
Co-authored-by: Jeremy Felder <[email protected]>
## Describe the changes This PR... ## Linked Issues Resolves #
yshekel
force-pushed
the
yshekel/V3
branch
2 times, most recently
from
0e2d8e2
to
73dd80b
Compare
|
|
||
| # TODO: bw6 on windows requires more memory than the standard runner has | ||
| # Add a large runner and then enable this job | ||
| # build-windows: |
There was a problem hiding this comment.
What about the Windows build? Should'nt we just remove this at this point?
| export DEFAULT_BACKEND_INSTALL_DIR=$PWD/../../../build/ | ||
| go test ./$FIELD/tests -count=1 -failfast -p 2 -timeout 60m -v | ||
|
|
||
| # build-hashes-linux: |
There was a problem hiding this comment.
are hashes not supported in v3 rn? This specific check only runs keccak
There was a problem hiding this comment.
we don't have hashes in v3 for now
There was a problem hiding this comment.
Lets remove this for now then
jeremyfelder
requested changes
Aug 26, 2024
Comment on lines
+4
to
+20
| type EIcicleError int | ||
|
|
||
| const ( | ||
| Success EIcicleError = iota // Operation completed successfully | ||
| InvalidDevice // The specified device is invalid | ||
| OutOfMemory // Memory allocation failed due to insufficient memory | ||
| InvalidPointer // The specified pointer is invalid | ||
| AllocationFailed // Memory allocation failed | ||
| DeallocationFailed // Memory deallocation failed | ||
| CopyFailed // Data copy operation failed | ||
| SynchronizationFailed // Device synchronization failed | ||
| StreamCreationFailed // Stream creation failed | ||
| StreamDestructionFailed // Stream destruction failed | ||
| ApiNotImplemented // The API is not implemented for a device | ||
| InvalidArgument // Invalid argument passed | ||
| UnknownError // An unknown error occurred | ||
| ) |
There was a problem hiding this comment.
It would be better to use the C.enum_eIcicleError values directly from cpp errors.h to prevent reordering issues.
See https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C for accessing C types from Go
yshekel
reviewed
Aug 29, 2024
| @@ -26,7 +26,7 @@ jobs: | |||
| uses: actions/checkout@v4 | |||
| - name: Check rustfmt | |||
| if: needs.check-changed-files.outputs.rust == 'true' || needs.check-changed-files.outputs.cpp_cuda == 'true' | |||
| working-directory: ./wrappers/rust | |||
| working-directory: ./wrappers/rust_v3 | |||
yshekel
reviewed
Aug 29, 2024
| @@ -0,0 +1,205 @@ | |||
| name: GoLang | |||
There was a problem hiding this comment.
rename to golang without v3_golang
yshekel
reviewed
Aug 29, 2024
| @@ -18,7 +18,9 @@ cpu_msm(const Device& device, const S* scalars, const A* bases, int msm_size, co | |||
| const S* batch_scalars = scalars + msm_size * batch_idx; | |||
| const A* batch_bases = config.are_bases_shared ? bases : bases + msm_size * batch_idx; | |||
| for (auto i = 0; i < msm_size; ++i) { | |||
| res = res + P::from_affine(batch_bases[i]) * batch_scalars[i]; | |||
| res = | |||
There was a problem hiding this comment.
can remove this now assuming fixed in new msm. Also note the icicle_v3 dir
|
Opened a new PR #594 with the same changes and review fixes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.