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.

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

Add the relaxed NAF #302

Merged
merged 10 commits into from
Feb 10, 2022
Merged

Add the relaxed NAF #302

merged 10 commits into from
Feb 10, 2022

Conversation

weikengchen
Copy link
Member

@weikengchen weikengchen commented Jul 4, 2021

Description

This PR adds the relaxed NAF, which differs from the original find_wnaf (should be find_naf?) by allowing the top 2 bits to be both nonzeros. This can, in some cases, reduce the overhead.

The algorithm is indeed quite simple. The only case when the standard NAF may be suboptimal (note: NAF is optimal for hamming weight, but not the length + the hamming weight) is when the NAF ends with "-1 0 1", which can be systematically changed into "1 1". There are no other cases.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@Pratyush
Copy link
Member

Pratyush commented Jul 7, 2021

Is this used anywhere? (Eg: for faster scalar muls?)

@weikengchen
Copy link
Member Author

weikengchen commented Jul 8, 2021

Is this used anywhere? (Eg: for faster scalar muls?)

I originally planned to use it for Poseidon. Unfortunately, for essentially all the prime numbers being considered, NAF is not better than the original form, because when NAF is better, it needs to use "inverse", and "inverse" here is costly, which requires 3 constraints to compute (it is needed to be compatible with the case that d is zero).

  • For all the primes <= 100, it is not better.
  • 127 seems to be the only one that is better by a little bit. But for Marlin, we often go to 257, which does not need "-1".
  • It becomes extremely useful when the number is very large, something like 47189, 3527, 8813 (randomly typing), but not the common prime numbers considered in Poseidon.

Maybe it would be more useful for EC scalar mul, in which "inverse" (aka negative) is cheap.

@weikengchen
Copy link
Member Author

On a separate note, should we rename find_wnaf to find_naf? there is no window.

@Pratyush
Copy link
Member

Pratyush commented Jul 8, 2021

Yeah, find_naf makes sense.

@Pratyush
Copy link
Member

Pratyush commented Feb 9, 2022

@weikengchen what's the status of this PR?

@weikengchen
Copy link
Member Author

I should be able to revisit this PR today or tomorrow.

@weikengchen
Copy link
Member Author

The PR is ready to go. @Pratyush please take a look

Comment on lines +151 to +152
let vec = find_relaxed_naf(&[12u64]);
assert_eq!(vec.len(), 4);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add here find_naf(&12u64]) which has a shorter length?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Added. It has a slightly longer len---5.

@Pratyush Pratyush merged commit 4dd6c34 into master Feb 10, 2022
@Pratyush Pratyush deleted the add-relaxed-naf branch February 10, 2022 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants