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 ´is_even(value, rc_bound)´ function to ´math_utils.py´ #65

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

omarespejel
Copy link

@omarespejel omarespejel commented May 21, 2022

Hey, looking to contribute and find this a necessity.

Description

Add a function that allows checking if a value is even.

Why?

Existing functions like merkle_update could benefit from such a check. Further functions (I am creating one to calculate the median value of an array) could benefit from distinguishing between even and odd values easily.

What

Created is_even(value, rc_bound) which allows:

  • asserts value is an integer
  • asserts value is within the (-rc_bound, rc_bound) range
  • returns true if value is even

Who could review?

Thank you and please let me know if any change is required.

@glrn @l-henri @liorgold2


This change is Reviewable

* asserts value is an integer
* asserts value is within the (-rc_bound, rc_bound) range
* returns true if value is even
Copy link
Contributor

@liorgold2 liorgold2 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @omarespejel)


src/starkware/cairo/common/math_utils.py line 26 at r1 (raw file):

    assert abs(val) < rc_bound, f"value={val} is out of the valid range."
    return val > 0

And a second blank line between the two functions.


src/starkware/cairo/common/math_utils.py line 27 at r1 (raw file):

    return val > 0

def is_even(value, rc_bound):

I wonder if this function is necessary. How do you intend to use it?


src/starkware/cairo/common/math_utils.py line 29 at r1 (raw file):

def is_even(value, rc_bound):
    """
    Returns true if the input is even and within the range (-rc_bound, rc_bound).

Use the same phrasing as above:

Returns True if the lift of the given field element, as an integer in the range
(-rc_bound, rc_bound), is even.

src/starkware/cairo/common/math_utils.py line 32 at r1 (raw file):

    Raises an exception if the element is not within the accepted range.
    """
    assert_integer(value)

You should use as_int as above to make sure that if value is, for example, PRIME - 1, it will be interpreted as -1 (otherwise, the function will fail on the "out of range" error instead of simply returning False)


src/starkware/cairo/common/math_utils.py line 34 at r1 (raw file):

    assert_integer(value)
    assert abs(value) < rc_bound, f"value={value} is out of the valid range."
    return value % 2 == 0

End file with the new-line character.

Copy link
Contributor

@liorgold2 liorgold2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @omarespejel)


src/starkware/cairo/common/math_utils.py line 27 at r1 (raw file):

Previously, liorgold2 wrote…

I wonder if this function is necessary. How do you intend to use it?

In particular, can you give a short code example taking advantage of this function?

@omarespejel omarespejel marked this pull request as draft May 22, 2022 06:52
@omarespejel
Copy link
Author

Thank you for your kind review @liorgold2! I applied the feedback. I will add a couple of examples of use cases in my next commit.

Meanwhile, I convert the PR to draft.

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.

2 participants