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 binary field initially #416

Merged
merged 10 commits into from
Jun 4, 2024
Merged

feat(math): implement binary field initially #416

merged 10 commits into from
Jun 4, 2024

Conversation

chokobole
Copy link
Contributor

Description

This PR implements BinaryField for binius.

See https://gitlab.com/IrreducibleOSS/binius

@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from 34f7b89 to 4f9f771 Compare May 16, 2024 04:56
TomTaehoonKim
TomTaehoonKim approved these changes May 16, 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

@chokobole chokobole marked this pull request as draft May 16, 2024 11:21
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch 2 times, most recently from 4c63976 to 285cb26 Compare May 28, 2024 06:12
@chokobole chokobole marked this pull request as ready for review May 28, 2024 06:13
dongchangYoo
dongchangYoo requested changes May 29, 2024
View reviewed changes
tachyon/math/finite_fields/generator/prime_field_generator/binary_prime_field_config.h.tpl Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_prime_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_fields/binary_fields.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_prime_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_tower_operations.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_prime_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/generator/prime_field_generator/binary_prime_field128_config.h.tpl Outdated Show resolved Hide resolved
docs/how_to_contribute/math/how_to_add_prime_fields.md Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from 285cb26 to 24dd738 Compare May 29, 2024 13:27
dongchangYoo
dongchangYoo approved these changes May 30, 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

Insun35
Insun35 requested changes May 30, 2024
View reviewed changes
tachyon/math/base/big_int.h Outdated Show resolved Hide resolved
tachyon/math/base/big_int.h Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from 24dd738 to 6a1026c Compare May 30, 2024 07:44
Insun35
Insun35 approved these changes May 31, 2024
View reviewed changes
Copy link
Contributor

@Insun35 Insun35 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 requested changes May 31, 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.

Please change the following commit messages!
e1f8e85
feat(math): implement BinaryField initially up to 8 bits
Also change for a4e9f33
390c5f5
Remove the "the"
feat(base): add (u)int128 cases to base::Range

tachyon/math/finite_fields/binary_fields/BUILD.bazel Show resolved Hide resolved
tachyon/math/finite_fields/prime_field_generator_unittest.cc Outdated Show resolved Hide resolved
tachyon/math/finite_fields/generator/prime_field_generator/build_defs.bzl Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_tower_operations.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_tower_operations.h Outdated Show resolved Hide resolved
tachyon/math/base/big_int.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/generator/prime_field_generator/build_defs.bzl Outdated Show resolved Hide resolved
tachyon/math/finite_fields/generator/generator_util.cc Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_prime_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/generator/prime_field_generator/prime_field_generator.cc Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from 6a1026c to 332a181 Compare May 31, 2024 12:40
ashjeong
ashjeong reviewed Jun 3, 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.

Could you change the commit titles' BinaryFields' to BinaryField?

tachyon/math/finite_fields/binary_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_field.h Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_fields/binary_fields.h Show resolved Hide resolved
tachyon/math/finite_fields/binary_fields/binary_fields.h Show resolved Hide resolved
tachyon/math/finite_fields/generator/prime_field_generator/binary_prime_field128_config.h.tpl Outdated Show resolved Hide resolved
docs/how_to_contribute/math/how_to_add_prime_fields.md Outdated Show resolved Hide resolved
docs/how_to_contribute/math/how_to_add_prime_fields.md Outdated Show resolved Hide resolved
docs/how_to_contribute/math/how_to_add_prime_fields.md Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_tower_operations.h Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch 3 times, most recently from 3c4f54d to cc07942 Compare June 3, 2024 05:30
ashjeong
ashjeong requested changes Jun 3, 2024
View reviewed changes
docs/how_to_contribute/math/how_to_add_prime_fields.md Outdated Show resolved Hide resolved
tachyon/math/finite_fields/binary_field.h Outdated Show resolved Hide resolved
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from cc07942 to 96ace9c Compare June 3, 2024 08:30
ashjeong
ashjeong approved these changes Jun 3, 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.

Will need to check again with std::optional Inverse changes. Else, LGTM!

@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch 2 times, most recently from e13d814 to 682558a Compare June 4, 2024 08:19
TomTaehoonKim
TomTaehoonKim approved these changes Jun 4, 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

@chokobole
feat(math): implement BinaryField initially up to 8 bits
442d3e5 
See
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1
and
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field_arithmetic.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1.
@chokobole
feat(math): implement BinaryField up to 64 bits
df89a43 
See
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1
and
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field_arithmetic.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1.
@chokobole
refac(base): use public absl::SimpleHexAtoi instead of internal fun…
9ff07ea 
…ction
@chokobole
feat(base): add (u)int128 cases to base::Range
5be69a5
@chokobole
feat(math): implement BinaryField for 128 bits
0f257b3 
See
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1
and
https://gitlab.com/IrreducibleOSS/binius/-/blob/main/crates/field/src/binary_field_arithmetic.rs?ref_type=85b1e2ab76f1f981484825f8893c9cdeb38ae5b1.
@chokobole
docs: capitalize elliptic curves
fa9e014
@chokobole
style: format how_to_add_prime_fields.md
a97ac55
@chokobole
build(math): stop generating prime field when use_montgomery and `r…
f85306d 
…educexx` are input together
@chokobole
docs: add the descriptions of the missing attributes
715af21
@chokobole
docs: update how_to_add_prime_fields.md
cbbe8ea
@chokobole chokobole force-pushed the feat/implement-binary-field-initially branch from 682558a to cbbe8ea Compare June 4, 2024 12:13
@chokobole chokobole merged commit 4838e85 into main Jun 4, 2024
3 checks passed
@chokobole chokobole deleted the feat/implement-binary-field-initially branch June 4, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongchangYoo dongchangYoo dongchangYoo approved these changes

@Insun35 Insun35 Insun35 approved these changes

@TomTaehoonKim TomTaehoonKim TomTaehoonKim 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
5 participants
@chokobole @dongchangYoo @Insun35 @TomTaehoonKim @ashjeong