Skip to content

Prototype for Probably Correct: Oscar issue 3110#2279

Draft
JohnAAbbott wants to merge 2 commits into
Nemocas:masterfrom
JohnAAbbott:JAA/ProbablyCorrect-1
Draft

Prototype for Probably Correct: Oscar issue 3110#2279
JohnAAbbott wants to merge 2 commits into
Nemocas:masterfrom
JohnAAbbott:JAA/ProbablyCorrect-1

Conversation

@JohnAAbbott
Copy link
Copy Markdown
Collaborator

@JohnAAbbott JohnAAbbott commented Apr 23, 2026

Prototype implementation incorporating some feedback.
To work this PR needs the code from the draft PR Nemocas/AbstractAlgebra.jl#2395

@JohnAAbbott JohnAAbbott added enhancement release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes needs AbstractAlgebra release labels Apr 23, 2026
@thofma
Copy link
Copy Markdown
Member

thofma commented Apr 23, 2026

What is the overhead of this? How much more expensive is next_prime(ZZ(2))?

@JohnAAbbott
Copy link
Copy Markdown
Collaborator Author

JohnAAbbott commented Apr 24, 2026

What is the overhead of this? How much more expensive is next_prime(ZZ(2))?

No idea! If you're calling next_prime(n) where n is ZZ(2) then you are already throwing away some performance compared to when n is Int64(2). I tried measuring the difference, and obtained a factor of 20, but there was over 60% GC time. Anyway, if dodgy mode is off then the overhead should be quite low.

I add that next_prime(::Int) is unchanged. Not sure how often one calls next_prime for value which do not fit into Int64.

@thofma
Copy link
Copy Markdown
Member

thofma commented Apr 24, 2026

Yes, my question was about the additional overhead when calling next_prime(ZZ(2)) with dodgy mode off.

@JohnAAbbott
Copy link
Copy Markdown
Collaborator Author

Yes, my question was about the additional overhead when calling next_prime(ZZ(2)) with dodgy mode off.

I did not observe any significant detrimental effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement needs AbstractAlgebra release release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants