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.

Sign up for GitHub

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

Jump to bottom

feat(math): implement packed field extension #478

Merged
merged 19 commits into from
Jul 22, 2024
Merged

feat(math): implement packed field extension #478

merged 19 commits into from
Jul 22, 2024

Conversation

chokobole
Copy link
Contributor

Description

This adds Extension Field of the Packed Field.

@chokobole chokobole force-pushed the feat/implement-packed-field-extension branch 2 times, most recently from bf2c02c to c78f40b Compare July 19, 2024 07:42
ashjeong
ashjeong reviewed Jul 19, 2024
View reviewed changes
Copy link
Contributor

@ashjeong ashjeong left a comment

Choose a reason for hiding this comment

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

6e337ae
Please change the commit title to be: "change constexpr variables to static member variables"
Fix the typo of “refactoring” in the commit message

5b5fcbc
Fix the typo in the commit title : "initialization"

@TomTaehoonKim
Copy link
Contributor

@chokobole Is this typo or did you intend? 0453008, rerepresent -> represent.

TomTaehoonKim
TomTaehoonKim approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@TomTaehoonKim TomTaehoonKim left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add reference of non_residues in the commit message body?

@chokobole chokobole force-pushed the feat/implement-packed-field-extension branch 2 times, most recently from de5c061 to 61c3bc0 Compare July 19, 2024 09:16
@ashjeong ashjeong force-pushed the feat/implement-packed-field-extension branch from 61c3bc0 to 197f07c Compare July 19, 2024 09:54
TomTaehoonKim
TomTaehoonKim approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@TomTaehoonKim TomTaehoonKim left a comment

Choose a reason for hiding this comment

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

LGTM

@ashjeong ashjeong force-pushed the feat/implement-packed-field-extension branch 2 times, most recently from 7bf0339 to 0920451 Compare July 21, 2024 15:03
ashjeong
ashjeong approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@ashjeong ashjeong left a comment

Choose a reason for hiding this comment

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

LGTM

GideokKim
GideokKim approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@GideokKim GideokKim left a comment

Choose a reason for hiding this comment

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

LGTM

@ashjeong
Copy link
Contributor

Should the FromBasePrimeFields() functions in quadratic, cubic, and quartic extension fields also be changed to FromBaseFields()?

@chokobole
Copy link
Contributor Author

Should the FromBasePrimeFields() functions in quadratic, cubic, and quartic extension fields also be changed to FromBaseFields()?

Exactly! GenerateInitExtField() should also be changed, but since they are not used, so I'll just leave a comment!

@chokobole chokobole force-pushed the feat/implement-packed-field-extension branch from 0920451 to ce13705 Compare July 22, 2024 05:36
dongchangYoo
dongchangYoo requested changes Jul 22, 2024
View reviewed changes
tachyon/math/finite_fields/fp3.h Outdated Show resolved Hide resolved
batzor
batzor approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@batzor batzor left a comment

Choose a reason for hiding this comment

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

LGTM

dongchangYoo
dongchangYoo approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@dongchangYoo dongchangYoo left a comment

Choose a reason for hiding this comment

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

LGTM

chokobole and others added 17 commits July 22, 2024 19:49
@chokobole
refac(math): use type size_t to represent the limb size
897fb8a
@chokobole
feat(math): add ExtensionDegree() to PackedPrimeFieldBase
0195cc1
@chokobole
feat(math): add FrobeniusMapInPlace() to PackedPrimeFieldBase
ac818f7
@chokobole
refac(math): change constexpr variables to static member variables
3a4e97f 
Since the constructor of the `PackedPrimeField` is not `constexpr`, this
refactoring is needed.
@chokobole
feat(math): enable extension packed field initialization
1613182
@chokobole
feat(math): use different initialization if the base prime field is p…
0936d5e 
…acked
@chokobole
feat(math): add PackedBabyBear4
c411748
@chokobole
feat(math): remove KoalaBear2
b6a3021
@chokobole
feat(math): add PackedKoalaBear4
7c49dcb
@chokobole
refac: rename PackedPrimeFieldTraits to PackedFieldTraits
7ee546e
@chokobole
refac: remove Prime from members of the PackedFieldTraits
d6e59f4
@chokobole
refac: allow to include goldilocks.h intead of `goldilocks_prime_fi…
849b656 
…eld.h`
@ashjeong @chokobole
feat(math): impl ExtendedPackedFieldTraits
713015b
@chokobole
feat(math): add ExtendedPackedFieldTraits for (Packed)BabyBear4
a01b535
@chokobole
feat(math): add ExtendedPackedFieldTraits for (Packed)KoalaBear4
0ad1c77
@chokobole
chore(math): add reference for non residue
65331a2
@chokobole
refac: add MaybeUnpack
e35feac
@chokobole chokobole force-pushed the feat/implement-packed-field-extension branch from ce887b6 to e35feac Compare July 22, 2024 11:11
@chokobole chokobole merged commit a9de5be into main Jul 22, 2024
3 checks passed
@chokobole chokobole deleted the feat/implement-packed-field-extension branch July 22, 2024 13:56
chokobole added a commit that referenced this pull request Aug 22, 2024
@chokobole
test: remove unneeded headers
d680d63 
See #478.
chokobole added a commit that referenced this pull request Aug 27, 2024
@chokobole
test: remove unneeded headers
0afdeb8 
See #478.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@batzor batzor batzor approved these changes

@dongchangYoo dongchangYoo dongchangYoo approved these changes

@TomTaehoonKim TomTaehoonKim TomTaehoonKim approved these changes

@GideokKim GideokKim GideokKim approved these changes

@ashjeong ashjeong ashjeong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@chokobole @TomTaehoonKim @ashjeong @batzor @dongchangYoo @GideokKim