Precompute #238
Precompute #238
Conversation
|
This is not building in ci, let us know if you want help diagnosing |
Codecov Report
@@ Coverage Diff @@
## dev #238 +/- ##
===================================================
+ Coverage 72.98368% 75.68389% +2.70021%
===================================================
Files 40 40
Lines 4290 4277 -13
Branches 717 703 -14
===================================================
+ Hits 3131 3237 +106
+ Misses 1021 885 -136
- Partials 138 155 +17 |
| pub fn __str__(&self) -> PyResult<String> { | ||
| Ok(format!("({}, {})",self.g1.into_affine().x, self.g1.into_affine().y)) | ||
| } | ||
|
|
||
| fn preprocess(&mut self, level: usize) -> PyResult<()> { |
There was a problem hiding this comment.
It's kinda nitpicky, but I think stylistically you should add some whitespace and reorder some lines to help readability. For example on this method--
fn preprocess(&mut self, level: usize) -> PyResult<()> {
self.pplevel = level;
//Everything requires a different kind of int (and only works with that kind)
let mut base: u64 = 2;
//calling pow on a u64 only accepts a u32 parameter for reasons undocumented
base = base.pow(level as u32);
let ppsize = (base as usize - 1) * (255 + level - 1) / (level);
self.pp = Vec::with_capacity(ppsize);
self.pp.push(self.g1.clone());
for i in 1..base-1
{
//Yes, I really need to expicitly cast the indexing variable...
let mut next = self.pp[i as usize -1].clone();
next.add_assign(&self.g1);
self.pp.push(next);
}
//FrRepr::from only takes a u64
let factor = Fr::from_repr(FrRepr::from(base)).unwrap();
//(x + y - 1) / y is a way to round up the integer division x/y
for i in base-1..(base - 1) * (255 + level as u64 - 1)/(level as u64) {
let mut next = self.pp[i as usize - (base-1) as usize].clone();
//Wait, so add_assign takes a borrowed object but mul_assign doesn't?!?!?!?
next.mul_assign(factor);
self.pp.push(next);
}
//It's not really Ok. This is terrible.
Ok(())
}
I think formalizing some of the comments would be a good idea too
pairing/src/lib.rs
Outdated
| }) | ||
| } | ||
|
|
||
| fn rand(&mut self, s1: u32, s2: u32, s3: u32, s4: u32) -> PyResult<()>{ | ||
| let mut rng = XorShiftRng::from_seed([s1,s2,s3,s4]); | ||
| let g = G1::rand(&mut rng); | ||
| self.g1 = g; | ||
| if self.pplevel != 0{ |
There was a problem hiding this comment.
I think it would be cleaner if you added a space after the conditional check in the if statements--
| if self.pplevel != 0{ | |
| if self.pplevel != 0 { | |
| ... | |
| } |
|
I'm happy to accept this now, but we also need to add more tests. The rust library itself includes a variety of tests, and I would like to have a similar level of coverage. It would also be good to double check that at least every operation is covered by the tests. There are quite a lot of conditional branches based on different types of inputs, lmul vs rmul, etc., and I don't think the coverage is too full yet. |
|
@tyurek Can you squash these commits and rebase? |
…ors in the betterpairing library
Code to do precomputation for elliptic curve point scalar multiplication (currently only for G1). Precomputation is tweakable through "precomputation levels" which determine how many elements to precompute. Specifically, (2^{level} - 1) * ceil(255/{level}) elements are precomputed, which should result in a speedup by a factor of roughly {level}. Note that the number of precompute elements is exponential in {level}, and so there's probably a point where this relation breaks down and memory accesses start to become more significant as the precomputed data can no longer be stored in cache.
Precomputation is integrated into G1, G2, and GT