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 issue on matrix construction over integer mod ring for large coefficients #39488

Merged
merged 2 commits into from
Feb 21, 2025

Conversation

HugoPasse
Copy link
Contributor

@HugoPasse HugoPasse commented Feb 10, 2025

Fixes #36104.

In previous versions of Python, int size was limited and therefore could be cast to long. With Python 3 this limit does not exists and the code breaks with large integers. We modify the construction of Matrix_modn_dense_template from Python integers, directly using some existing cast from Sage.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

@vneiger vneiger self-requested a review February 10, 2025 15:14
@vneiger vneiger added the sd128 tickets of Sage Days 128 Le Teich label Feb 10, 2025
Copy link
Contributor

@vneiger vneiger left a comment

Choose a reason for hiding this comment

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

looks good to me

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 11, 2025
sagemathgh-39488: Fix issue on matrix construction over integer mod ring for large coefficients
    
Fix Issue sagemath#36104.

In previous versions of Python, `int` size was limited and therefore
could be cast to `long`. With Python 3 this limit does not exists and
the code breaks with large integers. We modify the construction of
`Matrix_modn_dense_template`  from Python integers, directly using some
existing cast from Sage.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#39488
Reported by: Hugo Passe
Reviewer(s): Vincent Neiger
@DaveWitteMorris
Copy link
Member

Please change "Fix Issue #36104" to "Fixes issue #36104" at the start of the PR's description. Then the PR will be linked to the issue, so the issue will be listed under "Successfully merging this pull request may close these issues" at the right of this PR web page. (There will be a similar notation on the PR's web page) Also, the issue will close automatically when the PR is merged.

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 18, 2025
sagemathgh-39488: Fix issue on matrix construction over integer mod ring for large coefficients
    
Fixes sagemath#36104.

In previous versions of Python, `int` size was limited and therefore
could be cast to `long`. With Python 3 this limit does not exists and
the code breaks with large integers. We modify the construction of
`Matrix_modn_dense_template`  from Python integers, directly using some
existing cast from Sage.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#39488
Reported by: Hugo Passe
Reviewer(s): Vincent Neiger
@vbraun vbraun merged commit 8a44c73 into sagemath:develop Feb 21, 2025
13 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sd128 tickets of Sage Days 128 Le Teich
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Matrix construction over prime field fails for some types of inputs
4 participants