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

Allow CRT_list() to be called with one argument #38986

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented Nov 17, 2024

As in the title.

Opinion: should we allow crt() / CRT() to be called with one list also? Note that currently it is callable with two lists and does the exact same thing as CRT_list().

Opinion: is returning Mod(0, 1) for empty list a sane idea? Problem is later we may want to implement (pseudocode)

CRT_list([mod(x-1, x^2+1), mod(x+1, x^2-1)])

and in that case the correct thing to return is not clear when the list is empty.

More generally, if it's given elements x₁ ∈ R/I₁, …, xₙ ∈ R/Iₙ where Iᵢ = xᵢ.parent().defining_ideal() and all R = xᵢ.parent() are the same for all i, it should returns x ∈ R/(I₁.intersection(I₂).intersection(I₃).….intersection(Iₙ)). But this is not well-defined when values is empty.

📝 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. (not aware of one)
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

@user202729 user202729 changed the title Allow CRT_list() to be called with one element Allow CRT_list() to be called with one argument Nov 17, 2024
Copy link

github-actions bot commented Nov 17, 2024

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

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.

A few small things.

src/sage/arith/misc.py Outdated Show resolved Hide resolved
src/sage/arith/misc.py Outdated Show resolved Hide resolved
src/sage/arith/misc.py Outdated Show resolved Hide resolved
@user202729 user202729 requested a review from tscrim December 22, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants