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

Fix CyclotomicPol(501) and others #5728

Merged
merged 1 commit into from
May 28, 2024

Conversation

fingolfin
Copy link
Member

... by ensuring the content of CYCPOLCache is immutable, and CYCPOLCache.CPdiffodd returns a copy (so we can't accidentally modify the cache content)

PR #5707 / c1b6cf0 optimized CyclotomicPol. Afterwards the package distribution CI tests started failing while running the tests for FactInt. Investigation lead to this.

... by ensuring the content of CYCPOLCache is immutable, and
CYCPOLCache.CPdiffodd returns a copy (so we can't accidentally
modify the cache content)
@fingolfin fingolfin added regression A bug that only occurs in the branch, not in a release release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels May 27, 2024
@fingolfin fingolfin requested a review from frankluebeck May 27, 2024 18:56
@ChrisJefferson ChrisJefferson merged commit 19ae178 into gap-system:master May 28, 2024
24 of 25 checks passed
@ChrisJefferson
Copy link
Contributor

Oh, I only just noticed you asked @frankluebeck for a review. If @frankluebeck has any issues, I will un-merge the PR, so let us know if you have any concerns.

Copy link
Member

@frankluebeck frankluebeck left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this. This was a bug in #5728; the function should not return the object that is stored in the cache. (I would have used ShallowCopy instead of Immutable, but both is fine.)

@fingolfin fingolfin deleted the mh/fix-CyclotomicPol branch May 28, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression A bug that only occurs in the branch, not in a release release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants