#31414 closed enhancement (fixed)

validate input arguments of elimination_ideal

Reported by: Markus Wageringel Owned by:
Priority: minor Milestone: sage-9.5
Component: commutative algebra Keywords:
Cc: David Ayotte Merged in:
Authors: Markus Wageringel Reviewers: David Ayotte
Report Upstream: N/A Work issues:
Branch: 28c1e2a (Commits, GitHub, GitLab) Commit: 28c1e2a41633178a4c62d423c999c948a920a26c
Dependencies: #29979 Stopgaps:

Status badges

Description

As explained in 31367#comment:1, this ticket adds a check that all the arguments passed to elimination_ideal are actually variables of the ring, to avoid silent problems like this:

age: R.<x,y,z> = QQ[]
sage: R.ideal(x-y, z).elimination_ideal([y, R(0)])
Ideal (x - y, z) of Multivariate Polynomial Ring in x, y, z over Rational Field

Change History (7)

comment:1 Changed 22 months ago by Markus Wageringel

Authors: Markus Wageringel
Branch: u/gh-mwageringel/31414
Commit: 8ad70e6c63285b65b57b257c7f77d3a404173ad7
Status: newneeds_review

New commits:

0aba8a031367: fix elimination in case of quotient rings
8ad70e631414: validate input arguments of elimination_ideal

comment:2 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:3 Changed 17 months ago by David Ayotte

Cc: David Ayotte added
Reviewers: David Ayotte
Status: needs_reviewpositive_review

Everything looks good. I ran the doctest and everything passed except for an unrelated test in src/sage/rings/number_field/number_field_ideal.py which also fail on the develop branch (is there a ticket already open for this ?):

File "src/sage/rings/number_field/number_field_ideal.py", line 2199, in sage.rings.number_field.number_field_ideal.NumberFieldFractionalIdeal.invertible_residues
Failed example:
    list(K.ideal(8).invertible_residues())[:5]
Expected:
    [1, a - 1, -3*a, -2*a + 3, -a - 1]
Got:
    [1, a + 2, 3*a + 3, -2*a + 3, a]
**********************************************************************
1 item had failures:
   1 of  17 in sage.rings.number_field.number_field_ideal.NumberFieldFractionalIdeal.invertible_residues
    [685 tests, 1 failure, 6.16 s]

Note that I tested this on version 9.4.beta4.

comment:4 Changed 17 months ago by git

Commit: 8ad70e6c63285b65b57b257c7f77d3a404173ad728c1e2a41633178a4c62d423c999c948a920a26c
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. Last 10 new commits:

34a233dpull in 32117
cfe869bmodulo n better be for n > 0
3752b9csome more remaining tests
e6a697atruely ignore ignored bounds for ZZ.random_element
36c7c58pull 32124
be3e05f29979: a few more fixes
5f0f5dc29979: mark failing tests as not tested
bc1e14a29979: minor tweaks of doctests
5330692minor style error
28c1e2a31414: validate input arguments of elimination_ideal

comment:5 in reply to:  3 Changed 17 months ago by Markus Wageringel

Dependencies: #31367#29979
Status: needs_reviewpositive_review

Thank you for the review. To avoid a merge conflict, I have rebased onto #29979. Only the last commit is relevant to this ticket.

Replying to gh-DavidAyotte:

I ran the doctest and everything passed except for an unrelated test in src/sage/rings/number_field/number_field_ideal.py which also fail on the develop branch (is there a ticket already open for this ?):

I cannot reproduce it on my end, but there have been discussions about this problem in 31443#comment:119 and 30801#comment:171. Apparently, it depends on whether Pari comes from the system or not. I could not find a ticket specifically for this problem, though maybe I have missed it.

comment:6 Changed 16 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:7 Changed 15 months ago by Volker Braun

Branch: u/gh-mwageringel/3141428c1e2a41633178a4c62d423c999c948a920a26c
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.