optimizations for commitVector#43
Conversation
HayimShaul
force-pushed
the
bl12381-opt
branch
from
9434e59 to
96e46e5
Compare
Signed-off-by: Hayim Shaul <hayim.shaul@ibm.com>
HayimShaul
force-pushed
the
bl12381-opt
branch
from
96e46e5 to
fee269b
Compare
adecaro
left a comment
adecaro
left a comment
There was a problem hiding this comment.
Thanks for the submission.
Please, fix the DCO and add unit-test for the new function AddPairsOfProducts.
Signed-off-by: Hayim Shaul <hayim.shaul@ibm.com>
|
Hi @HayimShaul , it looks good to me apart from the position of the test function. |
Signed-off-by: Hayim Shaul <hayim.shaul@ibm.com>
ale-linux
left a comment
ale-linux
left a comment
There was a problem hiding this comment.
LGTM - just two minor nits
| func (p *CurveBase) AddPairsOfProducts(left []driver.Zr, right []driver.Zr, leftgen []driver.G1, rightgen []driver.G1) driver.G1 { | ||
| sum := leftgen[0].Mul2(left[0], rightgen[0], right[0]) | ||
| for i := 1; i < len(left); i++ { | ||
| sum.Add(leftgen[i].Mul2(left[i], rightgen[i], right[i])) |
There was a problem hiding this comment.
this function will panic if lengths are mismatched... @adecaro what do you think? We obviously can't return an error, so panic is right, how about a length check and a panic if it fails? Or are you concerned about performance?
There was a problem hiding this comment.
I think an if that panic with a better message is totally fine.
There was a problem hiding this comment.
I have just noticed that we don't have a check on ModAddMul though.
There was a problem hiding this comment.
so what's the conclusion?
add the check in AddPairsOfProducts and ModAddMul? or no need to check in both?
There was a problem hiding this comment.
I'd vote in favour of adding length checks and panic if they fail in all implementations of all vectored implementations
3 participants
No description provided.