-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: algebra #135
feat: algebra #135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments, nothing major.
Given those comments and the general structure, may I suggest the following refactor and generalization?
Consider creating an algebra
module with:
group
ring
field
As submodules. (You can leavering
as TODO).
Now, it is probably reasonable to then define the abstract notion of group
and field
and then use these as super traits for the more refined items such as FiniteGroup
, FiniteCyclicGroup
, FiniteField
, etc.
@Autoparallel Thanks for the review.
|
@lonerapier let me take a look. One thing we could also consider is making the ring/field Give me a sec to go look through this. |
src/algebra/README.md
Outdated
1. Closure: $a\oplus b=c\in G$ | ||
2. Associative: $(a\oplus b)\oplus c = a\oplus(b\oplus c)$ | ||
3. Existence of Identity element: $a\oplus 0 = 0\oplus a = a$ | ||
4. Existence of inverse element for every element of the set: $a\oplus b=0$ | ||
5. Commutativity: Groups which satisfy an additional property: *commutativity* on the set of | ||
elements are known as **Abelian groups**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nitpick, so don't feel compelled to change this (i love your documentation always) but I have a disdain for the
Instead consider just using concatenation:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to learn more abstract algebra :)
I think concatenation is mostly substituted as multiplication, at least in my head, so might create confusion. does using \cdot
make more sense?
Found it used in Ben Lynn's notes as well: https://crypto.stanford.edu/pbc/notes/group/group.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified it to
if not can just simplify it to concatenation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/algebra/README.md
Outdated
1. Abelian group under $\oplus$. | ||
2. Monoid under $\cdot$. | ||
3. Distributive with respect to $\oplus$. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again here
src/algebra/README.md
Outdated
2. Monoid under $\cdot$. | ||
3. Distributive with respect to $\oplus$. | ||
|
||
[Finite fields](https://en.wikipedia.org/wiki/Finite_field) are instrumental in cryptographic systems. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a touch here about how fields exist as special instances of rings (where there is division and no zero-divisors).
Adding a commented TODO is also okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, will change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Regarding this suggestion, I can't find a way to add this without complicating the traits and structs more Like it wouldn't be easy for a beginner to implement 5 traits just to have a PrimeField implementation |
That's true... it would introduce having some wrapper types and what not. The only benefit it would give is that if you did something like:
but that's probably not worth it. Another option is something like
but then I think we are hitting abstraction for the sake of abstraction. You're right, let's not make the implementation needs abusive to beginners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fantastic!
examples/permutation_group.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filename here is misleading, but if you are wanting to learn more abstract algebra, every finite group is a subgroup of a permutation (symmetric) group and the dihedral group is an especially nice subgroup of it which is implied by Lagrange's theorem
Inside of the dihedral group is then two cyclic groups. One of order n, and one of order 2. Also implied by lagrange's theorem.
If you don't implement those (don't have to at all), i'd suggest changing the filename :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though i guess in this case D6 is S3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have changed to symmetric_group
|
||
/// Defines a group with commutative operation: `a·b=b·a` | ||
pub trait AbelianGroup: Group { | ||
/// Returns whether the group is an abelian group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little misleading. This returns true if a pair of elements commutes.
Sadly an (almost) impossible task for Rust to handle is that there exists a proof that your impl of this trait is actually abelian (rip)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, IDK why I put it this way. Have changed this to an assertion
Maybe we can implement this as an example demonstration with Ring implementation. Like showing a ring with inverse and commutativity forms a field, and then comparing it with the actual Field implementation. |
It changes the following:
FiniteGroup
traitMultiplicativePrimeGroup
structCurveGroup
trait