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

Make sagemath with pari 2.17 more deterministic #4

Merged
merged 4 commits into from
Dec 24, 2024

Conversation

tornaria
Copy link

@tornaria tornaria commented Nov 8, 2024

This affects two number field functions which cause doctest unstability:

  1. reduced_basis() uses lll to reduce the basis; the new FLATTER algorithm in pari 2.17 is quite non-deterministic.
    Workaround: pass a flag to qflll() so pari does not use FLATTER.
  2. pari_bnf() this uses and caches the output of bnfinit() which I guess is also using lll to reduce the relations (bnf[4]) which causes non-determinism (fundamental units, finding generators of ideals, printing ideals).
    Workaround: use setrand(1) just before calling bnfinit(). It's hard to do this just on doctests since number fields are cached so the field may have been already constructed for another doctest way before the doctest that fails.

A third commit just adjusts doctests for the changed ideal display.

After this, it seems doctests are almost ok for pari 2.17 (in 64 bit / 32 bit, long / no long)

@antonio-rojas

src/sage/rings/number_field/S_unit_solver.py Show resolved Hide resolved
src/sage/rings/number_field/number_field.py Outdated Show resolved Hide resolved
src/sage/rings/number_field/number_field.py Outdated Show resolved Hide resolved
@antonio-rojas
Copy link
Owner

Ugh, sorry, I always forget I have to explicitly submit the reviews - i've had this in pending state for over a month

@antonio-rojas antonio-rojas merged commit ee8ee5a into antonio-rojas:pari-2.17 Dec 24, 2024
5 of 15 checks passed
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