chore: overhead free type conversion for load_poly_to_gpu

[mrain]

Description

Update: this also fixes the API changes when gpu_vid is enabled.

This PR avoids a memory copy in load_poly_to_gpu. However it contains unsafe rust code.

Before:

Start:   KZG10::Setup with prover degree 4194304 and verifier degree 1
...
Start:   Type Conversion: ark->ICICLE: Scalar
End:     Type Conversion: ark->ICICLE: Scalar ......................................17.305ms
...

After:

Start:   KZG10::Setup with prover degree 4194304 and verifier degree 1
...
Start:   Type Conversion: ark->ICICLE: Scalar
End:     Type Conversion: ark->ICICLE: Scalar ......................................160ns
...

It doesn't apply to affine point conversion because of two different underlying struct reprs.

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (main)
• Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the GitHub PR explorer

[alxiong]

This is pretty impressive speed-up, but you pushed the instance to be 2^22 to make the gap meaningful, for much smaller instances (which is our case), the gap is so small, (correct?)

• I have no objection of using this unsafe, since it's very limited scope, and easy to reason about. As long as we add some tests to constantly watching for the "same underlying repr" assumption is correct, I'm comfortable. (ICICLE is using tones of unsafe anyway.)
• I vote we turn this into a draft PR and archive it (instead of merging it). and use it in the future when the instance size become so large that conversion time is a bottleneck.

wdyt? @chancharles92 @mrain

[ggutoski]

I like this test. But can we do more? Earlier code comment says:

We assume that two types use the same underline repr.

In that case, we should be able to assert that the bytes are exactly the same. That way, if an incompatibility is introduced then the test will detect it immediately, no?

[mrain]

This is a bit annoying but doable: c82e230

[ggutoski]

Thank you! I don't fully understand this new test but here's what I see:

1. make a slice of random new ark scalars in scalars
2. convert them into a IC scalars slice ic_scalars via from_ark.
3. copy the slice into a new GPU slice called d_scalars.
4. transform the GPU slice to Montgomery form on the GPU
5. copy the GPU scalars out of GPU and back into ic_scalars
6. these ic_scalars (in Montgomery form) should be byte-for-byte identical to the original scalars. Check this via align_to

Not sure why you need to copy to/from GPU but AFIACT this test does what it's supposed to.

[ggutoski]

• I have no objection of using this unsafe, since it's very limited scope, and easy to reason about. As long as we add some tests to constantly watching for the "same underlying repr" assumption is correct, I'm comfortable. (ICICLE is using tones of unsafe anyway.)

We could add a test to enforce this, no? See #516 (comment)

• I vote we turn this into a draft PR and archive it (instead of merging it). and use it in the future when the instance size become so large that conversion time is a bottleneck.

I think I disagree. An archived PR will experience bit rot and get forgotten. Is your concern only about the unsafe code? Perhaps that concern could be mitigated with a stricter test described above.