Opened 22 months ago
Closed 15 months ago
#31414 closed enhancement (fixed)
validate input arguments of elimination_ideal
Reported by:  Markus Wageringel  Owned by:  

Priority:  minor  Milestone:  sage9.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: 
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(xy, 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
Authors:  → Markus Wageringel 

Branch:  → u/ghmwageringel/31414 
Commit:  → 8ad70e6c63285b65b57b257c7f77d3a404173ad7 
Status:  new → needs_review 
comment:2 Changed 21 months ago by
Milestone:  sage9.3 → sage9.4 

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:3 followup: 5 Changed 17 months ago by
Cc:  David Ayotte added 

Reviewers:  → David Ayotte 
Status:  needs_review → positive_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
Commit:  8ad70e6c63285b65b57b257c7f77d3a404173ad7 → 28c1e2a41633178a4c62d423c999c948a920a26c 

Status:  positive_review → needs_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:
34a233d  pull in 32117

cfe869b  modulo n better be for n > 0

3752b9c  some more remaining tests

e6a697a  truely ignore ignored bounds for ZZ.random_element

36c7c58  pull 32124

be3e05f  29979: a few more fixes

5f0f5dc  29979: mark failing tests as not tested

bc1e14a  29979: minor tweaks of doctests

5330692  minor style error

28c1e2a  31414: validate input arguments of elimination_ideal

comment:5 Changed 17 months ago by
Dependencies:  #31367 → #29979 

Status:  needs_review → positive_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 ghDavidAyotte:
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
Milestone:  sage9.4 → sage9.5 

comment:7 Changed 15 months ago by
Branch:  u/ghmwageringel/31414 → 28c1e2a41633178a4c62d423c999c948a920a26c 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
31367: fix elimination in case of quotient rings
31414: validate input arguments of elimination_ideal