Skip to content

Raise an error when generating random integers when Height option is nonpositive #3893

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

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

d-torrance
Copy link
Member

@d-torrance d-torrance commented Jun 20, 2025

This fixes #2089. Also, we now raise an error when the height is negative instead of silently taking the absolute value (which was undocumented). The proposed behavior also matches the current behavior for random QQ's, not to mention random(ZZ), which yells if the input isn't positive.

Before

i1 : random(ZZ^5, ZZ^5, Height => -20)

o1 = | 13 10 11 3  13 |
     | 8  1  12 13 11 |
     | 16 0  1  1  7  |
     | 9  5  8  9  8  |
     | 1  16 12 15 8  |

              5       5
o1 : Matrix ZZ  <-- ZZ

i2 : random(ZZ^5, ZZ^5, Height => 0)
Floating point exception (core dumped)

Process M2 exited abnormally with code 136

After

i1 : random(ZZ^5, ZZ^5, Height => -20)
stdio:1:6:(3): error: expected a positive height

i2 : random(ZZ^5, ZZ^5, Height => 0)
stdio:2:6:(3): error: expected a positive height

@d-torrance d-torrance requested a review from mikestillman June 20, 2025 18:14
We already do this for random QQ's.  This will prevent us from
leaking a ZZ and also allow us to throw errors.
Previously, we silently ignored the negative sign when height was
negative, but this was undocumented, and crashed when the height was
zero.
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.

1 participant