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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 7, 2025
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f4c663
Split rawSetRandomInteger out from rawRandomInteger
d-torrance Jun 20, 2025
6acfc06
Raise error if random(ZZ) called w/ nonpositive height
d-torrance Jun 20, 2025
13c5758
Add comments about raw random integer functions & garbage collection
d-torrance Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions M2/Macaulay2/d/interface.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ setupfun("testCatch",testCatch);

export rawRandomZZ(e:Expr):Expr := (
when e
is Nothing do toExpr(Ccode(ZZ, "rawRandomInteger(", "NULL)"))
is maxN:ZZcell do toExpr(Ccode(ZZ, "rawRandomInteger(", maxN.v, ")"))
is Nothing do toExpr(Ccode(ZZorNull, "rawRandomInteger(", "NULL)"))
is maxN:ZZcell do toExpr(Ccode(ZZorNull, "rawRandomInteger(", maxN.v, ")"))
else WrongArgZZ());
setupfun("rawRandomZZ",rawRandomZZ);
export rawFareyApproximation(e:Expr):Expr := (
Expand Down
7 changes: 6 additions & 1 deletion M2/Macaulay2/e/ZZ.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ std::pair<bool, long> RingZZ::coerceToLongInteger(ring_elem a) const
mpz_get_si(a.get_mpz()));
}

ring_elem RingZZ::random() const { return ring_elem(rawRandomInteger(nullptr)); }
ring_elem RingZZ::random() const {
mpz_ptr result = new_elem();
rawSetRandomInteger(result, nullptr);
mpz_reallocate_limbs(result);
return ring_elem(result);
}

void RingZZ::elem_text_out(buffer &o,
const ring_elem ap,
Expand Down
3 changes: 1 addition & 2 deletions M2/Macaulay2/e/aring-zz-gmp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ class ARingZZGMP : public SimpleARing<ARingZZGMP>
void swap(ElementType& a, ElementType& b) const { mpz_swap(&a, &b); }
void random(ElementType& result) const
{
// TODO: this leaks a gmp_ZZ
mpz_set(&result, rawRandomInteger(nullptr));
rawSetRandomInteger(&result, nullptr);
}
/** @} */

Expand Down
26 changes: 18 additions & 8 deletions M2/Macaulay2/e/interface/random.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,29 @@ int32_t rawRandomInt(int32_t max)
return RandomSeed % max;
}

void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN)
/* if height is the null pointer, use the default height */
{
if (maxN == nullptr) maxN = maxHeight;
if (mpz_cmp_si(maxN, 0) <= 0)
throw exc::engine_error("expected a positive height");

mpz_urandomm(result, state, maxN);
}

gmp_ZZ rawRandomInteger(gmp_ZZ maxN)
/* if height is the null pointer, use the default height */
{
mpz_ptr result = getmemstructtype(mpz_ptr);
mpz_init(result);
if (maxN == nullptr)
mpz_urandomm(result, state, maxHeight);
else if (1 != mpz_sgn(maxN))
{
mpz_set_si(result, 0);
}
else
mpz_urandomm(result, state, maxN);

try {
rawSetRandomInteger(result, maxN);
} catch (const exc::engine_error& e) {
ERROR(e.what());
return nullptr;
}

mpz_reallocate_limbs(result);
return result;
}
Expand Down
3 changes: 3 additions & 0 deletions M2/Macaulay2/e/interface/random.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ unsigned long rawRandomULong(unsigned long max);
int32_t rawRandomInt(int32_t max);
/* generate a random number in the range 0..max-1 */

void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN);
/* if height is the null pointer, use the default height */

gmp_ZZ rawRandomInteger(gmp_ZZ maxN);
/* if height is the null pointer, use the default height */
Copy link
Member Author

@d-torrance d-torrance Nov 7, 2025

Choose a reason for hiding this comment

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

Add a comment: rawRandomInteger returns garbage-collected memory, and rawSetRandomInteger doesn't deal w/ garbage collection


Expand Down
3 changes: 3 additions & 0 deletions M2/Macaulay2/tests/normal/randommat.m2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ assert isSurjective random(R^3,R^6,MaximalRank=>true)
assert(random(ZZ^2, ZZ^2, MaximalRank => true) - id_(ZZ^2) != 0)
assert(random(QQ^2, QQ^2, MaximalRank => true) - id_(QQ^2) != 0)
assert(random(R^2, R^2, MaximalRank => true) - id_(R^2) != 0)

-- used to crash M2 (#2089)
assert try random(ZZ^2, ZZ^2, Height => 0) then false else true