Skip to content

Implement crt for multivariate polynomials #40117

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

Merged
merged 6 commits into from
Jun 14, 2025

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented May 17, 2025

Mirroring the method with same name in Integer class.

Implementation detail: The method lift could probably be moved up to Modules.ElementMethods, but if we do that it should probably have a better name such as as_linear_combination.

Possible follow-up: make this work with CRT_list free function too.

📝 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.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Some minor comments, otherwise LGTM.

@user202729
Copy link
Contributor Author

The GitHub Actions can definitely be improved in this case.

Copy link

github-actions bot commented Jun 2, 2025

Documentation preview for this PR (built with commit 0c58cee; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 4, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 6, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 8, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 9, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 9, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 9, 2025
sagemathgh-40117: Implement crt for multivariate polynomials
    
Mirroring the method with same name in `Integer` class.

Implementation detail: The method `lift` could probably be moved up to
`Modules.ElementMethods`, but if we do that it should probably have a
better name such as `as_linear_combination`.

Possible follow-up: make this work with `CRT_list` free function too.

### 📝 Checklist


- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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#40117
Reported by: user202729
Reviewer(s): Travis Scrimshaw, user202729
@vbraun vbraun merged commit eb1bc9f into sagemath:develop Jun 14, 2025
20 of 22 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.

3 participants