Skip to content

Conversation

yshekel
Copy link
Collaborator

@yshekel yshekel commented Jun 18, 2025

  • Introduce reinterpret_slice and reinterpret_slice_mut for view casting HostOrDeviceSlice between any two types
  • Implement flatten_polyring_slice and flatten_polyring_slice_mut to abstract flattening of HostOrDeviceSlice<P> to HostOrDeviceSlice<P::Base>
  • Add generic polyvec_add, polyvec_sub, polyvec_mul, polyvec_mul_by_scalar, and polyvec_sum_reduce over HostOrDeviceSlice<PolynomialRing>
  • Use flattening logic to delegate polynomial operations to existing base field vector ops

cuda-backend-branch: main
metal-backend-branch: main

@yshekel yshekel requested a review from ChickenLover June 18, 2025 14:39
@yshekel yshekel changed the title [DRAFT] [FEAT] Vecops for Polynomial rings [FEAT] Vecops for Polynomial rings Jun 19, 2025
@yshekel yshekel changed the title [FEAT] Vecops for Polynomial rings [FEAT] Add reinterpretation utilities and polynomial vector operations Jun 19, 2025
@yshekel yshekel marked this pull request as ready for review June 19, 2025 11:22
let from_size = size_of::<From>();
let to_size = size_of::<To>();

assert!(from_size > 0 && to_size > 0, "Invalid zero-sized types");
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use Result here instead of assertion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good point, I will make the reinterpret return a Result

}

fn memset(&mut self, _: u8, _: usize) -> Result<(), eIcicleError> {
panic!("Cannot memset immutable UnifiedSlice");
Copy link
Contributor

Choose a reason for hiding this comment

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

Result here is returned, but panic is used inside

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will fix

Copy link
Contributor

@nonam3e nonam3e left a comment

Choose a reason for hiding this comment

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

everything is fine, but maybe it is better to remove panics where it's possible

@yshekel
Copy link
Collaborator Author

yshekel commented Jun 19, 2025

everything is fine, but maybe it is better to remove panics where it's possible

Great observation. I refactored and return Results now so errors returned to used instead of panic

@yshekel yshekel requested a review from nonam3e June 19, 2025 14:10
Copy link
Contributor

@LeonHibnik LeonHibnik left a comment

Choose a reason for hiding this comment

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

Nice
TODO add more error types in eIcicileError

@yshekel yshekel merged commit 6dda712 into main Jun 22, 2025
9 of 19 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