#9049 closed defect (fixed)
v4.4.1 bug in variety() over finite field extensions of Q?
Reported by: | cynthia_vinzant | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | linear algebra | Keywords: | |
Cc: | Merged in: | sage-4.6.alpha2 | |
Authors: | Andrey Novoseltsev | Reviewers: | Burcin Erocal |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I'm interested in computing the 0-dimensional variety of an ideal over finite field extensions of Q. I've tried the following code both on my copy (v4.4.1) of sage and online and it produces an error message. My friend tried the same code on his v4.2 sage and it worked fine. Is it possible there's a bug in the newer version?
S.<t>=PolynomialRing(QQ) F.<q>=QQ.extension(t^4+1) R.<x,y>=PolynomialRing(F) I=R.ideal(x,y^4+1) I.variety()
This produces the following error message:
Traceback (click to the left of this block for traceback) ... ValueError: Length must be equal to the degree of this number field Traceback (most recent call last): File "<stdin>", line 1, in <module> File "_sage_input_11.py", line 10, in <module> exec compile(u'open("___code___.py","w").write("# -*- coding: utf-8 -*-\\n" + _support_.preparse_worksheet_cell(base64.b64decode("Uy48dD49UG9seW5vbWlhbFJpbmcoUVEpCkYuPHE+PVFRLmV4dGVuc2lvbih0XjQrMSkKUi48eCx5Pj1Qb2x5bm9taWFsUmluZyhGKQpJPVIuaWRlYWwoeCx5XjQrMSkKSS52YXJpZXR5KCk="),globals())+"\\n"); execfile(os.path.abspath("___code___.py")) File "", line 1, in <module> File "/tmp/tmpZcwb0q/___code___.py", line 7, in <module> exec compile(u'I.variety() File "", line 1, in <module> File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 407, in __call__ return self.f(self._instance, *args, **kwds) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 2094, in variety TI = self.triangular_decomposition('singular:triangLfak') File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 407, in __call__ return self.f(self._instance, *args, **kwds) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 901, in triangular_decomposition is_groebner = self.basis_is_groebner() File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/misc/cachefunc.py", line 322, in __call__ return self._cachedmethod._instance_call(self._instance, *args, **kwds) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/misc/cachefunc.py", line 466, in _instance_call cache[key] = self._cachedfunc.f(inst, *args, **kwds) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_ideal.py", line 1666, in basis_is_groebner F = matrix(R, 1, self.ngens(), self.gens()) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/matrix/constructor.py", line 652, in matrix return matrix_space.MatrixSpace(ring, nrows, ncols, sparse=sparse)(entries) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/matrix/matrix_space.py", line 405, in __call__ return self.matrix(entries, copy=copy, coerce=coerce, rows=rows) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/matrix/matrix_space.py", line 1136, in matrix return self.__matrix_class(self, entries=x, copy=copy, coerce=coerce) File "matrix_generic_dense.pyx", line 68, in sage.matrix.matrix_generic_dense.Matrix_generic_dense.__init__ (sage/matrix/matrix_generic_dense.c:1997) File "multi_polynomial_libsingular.pyx", line 758, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:7176) File "parent.pyx", line 854, in sage.structure.parent.Parent.__call__ (sage/structure/parent.c:6332) File "coerce_maps.pyx", line 82, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3108) File "coerce_maps.pyx", line 77, in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (sage/structure/coerce_maps.c:3010) File "/usr/local/sage2/local/lib/python2.6/site-packages/sage/rings/number_field/number_field.py", line 1023, in _element_constructor_ raise ValueError, "Length must be equal to the degree of this number field" ValueError: Length must be equal to the degree of this number field
Attachments (3)
Change History (19)
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
I can verify that the bug still exists in v4.4.3. This seems to be because of I.gens() is returning a tuple, while I.basis_is_groebner, specifically the line
F = matrix(R, 1, self.ngens(), self.gens())
expects I.gens() to be a list. Did I.gens() change its return type recently? A quick hack converting self.gens() to a list solves the problem. Should I provide that patch?
comment:4 Changed 10 years ago by
Should I provide that patch?
That would be great. Thanks!
comment:5 Changed 10 years ago by
Thanks very much -- it works wonderfully.
comment:6 Changed 9 years ago by
- Milestone set to sage-4.6.1
- Status changed from new to needs_review
comment:7 Changed 9 years ago by
- Component changed from algebraic geometry to linear algebra
While the previous patch works for the problem of this ticket, it does not solve the real issue - you can construct a matrix from a list, but not from a tuple of the same elements, which does not make much sense. The new patch changes the relevant matrix constructor to allow tuples.
comment:8 Changed 9 years ago by
- Reviewers set to Burcin Erocal
- Status changed from needs_review to needs_work
Can you provide a more direct test to check the case when entries
is a tuple
?
Perhaps the example given in this ticket should be placed in the .variety()
method of the ideal class. It is not guaranteed that we will always use the Matrix_generic_dense
class in the future.
comment:10 Changed 9 years ago by
Thanks! That was quick. :)
I suggest to replace the check
if not isinstance(entries, list):
with
if not isinstance(entries, (list, tuple)):
instead of accepting ValueError
s as well.
attachment:trac_9049_bug_in_matrices_from_tuples.take2.patch includes this alternative approach.
Please switch this to positive_review
if you agree with my changes.
comment:11 Changed 9 years ago by
I thought about it, but I was not sure if it is necessary somewhere later to have exactly list, rather than tuple. Also, since the point of this try-block is to see whether or not it is possible to perform certain conversion, I think that any exception resulting from "wrong" conversion can be intercepted. If we just skip it for tuples, will we need later to skip it for, say, sequences? So I would prefer to stick with my patch as I think it is more universal. What do you think?
comment:12 Changed 9 years ago by
Sequence
s are lists:
sage: t = Sequence([1..5]) sage: isinstance(t, list) True
Explicitly checking for the condition you are testing is better than trial and error. You cannot know the meaning of the ValueError
returned from the base rings __call__
method, especially in such a generic setting.
IMHO, that try
& except
block should be cleaned up. However it's hard to do so as it is, since this is a generic constructor, there are no doctests or specification of what the acceptable input is and doctesting the whole sage library takes hours on my laptop.
Please reconsider my suggestion, with the "better safe then sorry" motto in mind.
comment:13 Changed 9 years ago by
- Status changed from needs_review to positive_review
OK, let's use the alternative patch! Switching to positive review.
comment:14 Changed 9 years ago by
- Merged in set to sage-4.6.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 9 years ago by
- Milestone changed from sage-4.6.1 to sage-4.6
comment:16 Changed 4 years ago by
- Description modified (diff)
I just made the error message to be a code block.