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

PackedFelt252Packed27 #484

Open
wants to merge 1 commit into
base: gali/rc_for_poseidon
Choose a base branch
from

Conversation

Gali-StarkWare
Copy link
Contributor

@Gali-StarkWare Gali-StarkWare commented Feb 9, 2025

This change is Reviewable

Copy link
Contributor Author

Gali-StarkWare commented Feb 9, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

@ohad-starkware ohad-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)


stwo_cairo_prover/crates/prover_types/src/simd.rs line 449 at r1 (raw file):

}

pub struct PackedFelt252Packed27 {

this name is kind of confusing no?

Copy link
Contributor Author

@Gali-StarkWare Gali-StarkWare left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ohad-starkware and @shaharsamocha7)


stwo_cairo_prover/crates/prover_types/src/simd.rs line 449 at r1 (raw file):

Previously, ohad-starkware (Ohad) wrote…

this name is kind of confusing no?

I agree, this is how it's auto-generated. We could change the auto-gen code in this case, should we name it PackedFelt252_27? Or something else without the underscore (I don't like it but I wanted to loose the second packed)

Copy link
Contributor

@ohad-starkware ohad-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Gali-StarkWare and @shaharsamocha7)


stwo_cairo_prover/crates/prover_types/src/simd.rs line 449 at r1 (raw file):

Previously, Gali-StarkWare wrote…

I agree, this is how it's auto-generated. We could change the auto-gen code in this case, should we name it PackedFelt252_27? Or something else without the underscore (I don't like it but I wanted to loose the second packed)

I think the cpu type is the issue, maybe ask DanC if he has ideas to change it?

@Gali-StarkWare Gali-StarkWare force-pushed the gali/packed_felt252packed27 branch 2 times, most recently from 04c2ab8 to 52347d6 Compare February 13, 2025 15:11
@Gali-StarkWare Gali-StarkWare force-pushed the gali/packed_felt252packed27 branch from 52347d6 to e12310e Compare February 13, 2025 15:26
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