Opened 5 years ago

Closed 5 years ago

#17205 closed defect (fixed)

update ore_algebra package

Reported by: zieglerk Owned by:
Priority: major Milestone: sage-6.4
Component: packages: optional Keywords: ImportError
Cc: fredrik.johansson, mkauers, schilly Merged in:
Authors: Manuel Kauers Reviewers: Karl-Dieter Crisman, John Palmieri, Konstantin Ziegler
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jhpalmieri)

After installing the ore_algebra-0.1 in Sage 6.3, the import as suggested in the documentation fails.

sage: from ore_algebra import *
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-27ab6e1c2b69> in <module>()
----> 1 from ore_algebra import *

/home/zieglerk/local/share/sage/local/lib/python2.7/site-packages/ore_algebra/__init__.py in <module>()
     12 
     13 from ore_algebra import OreAlgebra
---> 14 from guessing import guess, guess_raw
     15 
     16 """

/home/zieglerk/local/share/sage/local/lib/python2.7/site-packages/ore_algebra/guessing.py in <module>()
     30 from sage.rings.integer_ring import ZZ
     31 from sage.rings.rational_field import QQ
---> 32 from sage.rings.finite_rings.all import GF, is_FiniteField
     33 from sage.matrix.constructor import Matrix, matrix
     34 from sage.rings.arith import xgcd

ImportError: cannot import name is_FiniteField

Similarly on SageMathCloud running Sage 6.3.

from ore_algebra import *
Error in lines 1-1
Traceback (most recent call last):
  File "/projects/9700d68d-67c2-4a91-a689-c1d5b8b3f15e/.sagemathcloud/sage_server.py", line 841, in execute
    exec compile(block+'\n', '', 'single') in namespace, locals
  File "", line 1, in <module>
  File "/usr/local/sage/sage-6.3.beta6/local/lib/python2.7/site-packages/ore_algebra/__init__.py", line 14, in <module>
    from guessing import guess, guess_raw
  File "/usr/local/sage/sage-6.3.beta6/local/lib/python2.7/site-packages/ore_algebra/guessing.py", line 32, in <module>
    from sage.rings.finite_rings.all import GF, is_FiniteField
ImportError: cannot import name is_FiniteField

New spkg: http://www.risc.jku.at/research/combinat/software/ore_algebra/ore_algebra-0.2.spkg

This should be copied to the mirrors. Since it is an old-style spkg, no modifications to Sage are needed.

Change History (23)

comment:1 Changed 5 years ago by zieglerk

  • Description modified (diff)

comment:2 Changed 5 years ago by jhpalmieri

It looks like the ore_algebra package needs to be changed: is_FiniteField is no longer available as an import from sage.rings.finite_rings.all. This change happened between version 6.2 and 6.3. (The function is_FiniteField has been deprecated for some time, so this change should not have been a surprise.)

You should be able to fix this by changing the line

from sage.rings.finite_rings.all import GF, is_FiniteField

in the file /home/zieglerk/local/share/sage/local/lib/python2.7/site-packages/ore_algebra/guessing.py to

from sage.rings.finite_rings.all import GF

comment:3 Changed 5 years ago by kcrisman

  • Report Upstream changed from N/A to Fixed upstream, in a later stable release.
  • Summary changed from ore_algebra fails to import to update ore_algebra package

Looks like upstream is ahead of us and just didn't bother to mention it! This says it's at 0.2, for Sage 6.3.

comment:4 Changed 5 years ago by jhpalmieri

  • Authors set to Karl-Dieter Crisman
  • Description modified (diff)
  • Reviewers set to John Palmieri
  • Status changed from new to needs_review

So we should be able to solve this by upgrading to version 0.2. I took a look at 0.2, and it imports without breaking. Not everything works, though, and I wish it had functioning doctests or a functioning test suite, or both. A quick example, taken from the file nullspace.py:

sage: from ore_algebra.nullspace import *
sage: my_solver = cra(kronecker(gauss()))
sage: A = MatrixSpace(ZZ['x', 'y'], 4, 5).random_element()
sage: V = my_solver(A)

produces a TypeError for me (with Sage 6.5.beta0):

TypeError: x must be one of the generators of the parent.

Oh well. Let's upgrade on this ticket. Later on, we might consider changing this to a new-style spkg, but that can wait.

Shall I give this a positive review?

Last edited 5 years ago by jhpalmieri (previous) (diff)

comment:5 Changed 5 years ago by jhpalmieri

(I don't really know who should be the "author" for this ticket. kcrisman found the new version, so I've listed him.)

comment:6 Changed 5 years ago by kcrisman

  • Cc fredrik.johansson mkauers added

I haven't really thought about this, but if it doesn't even work properly, maybe this should become an experimental package?

comment:7 Changed 5 years ago by jhpalmieri

I thought about that option, too.

comment:8 Changed 5 years ago by zieglerk

I just installed version 2.0 of the ore_algebra package in the latest development version of Sage (6.5.beta0). It loads without errors, but naturally I can't doctest without the sources. For what it's worth, the initial examples of the documentation work as described.

comment:9 follow-up: Changed 5 years ago by jhpalmieri

The sources are present in SAGE_ROOT/local/lib/python/site-libraries/ore_algebra. Just running doctests on the files there doesn't work, though, I think because they're not part of the official Sage library. At least it didn't work for me. You can cut-and-paste examples from the source code into a Sage session to test it; that's what I did with nullspace.py (although you will need to add some import statements here and there).

Last edited 5 years ago by jhpalmieri (previous) (diff)

comment:10 follow-up: Changed 5 years ago by kcrisman

  • Reviewers changed from John Palmieri to John Palmieri, Konstantin Ziegler

I guess enough of them work that we can use it, but it would be "really nice" (hint hint) if the people responsible for it would let us know why some stuff isn't working...

comment:11 in reply to: ↑ 9 Changed 5 years ago by zieglerk

Replying to jhpalmieri:

The sources are present in SAGE_ROOT/local/lib/python/site-libraries/ore_algebra. Just running doctests on the files there doesn't work, though, I think because they're not part of the official Sage library.

Thanks for the clarification. I saw those files, but interpreted the instructions about Doctesting (paragraph "but not") as request to look exclusively into SAGE_ROOT/src/...

comment:12 in reply to: ↑ 10 Changed 5 years ago by mkauers

Replying to kcrisman:

I guess enough of them work that we can use it, but it would be "really nice" (hint hint) if the people responsible for it would let us know why some stuff isn't working...

Hi there!

I have been using Sage 6.1.1 until a few weeks ago, and version 0.1 of the package works properly in this version, at least for me. For Sage 6.3, as far as I see the only necessary change was the new location of is_FiniteField. With this being fixed in version 0.2, I am not aware of other problems for 6.3. If you know of any, I'd be happy to hear about them so that I can fix them too. Naturally, without having access to 6.5.beta0, I can't easily see what causes the TypeError? in the nullspace module.

In general, I must admit that the general workflow is not really clear to me. Is it explained somewhere what would be the right steps for me to take if I want to propagate updates I do to the ore_algebra package regularly to the version which is included in Sage? I also don't know what "new-style" packages are.

Thanks & Greetings, Manuel

comment:13 Changed 5 years ago by kcrisman

Hi, Manuel! I think that whether you (as upstream) want to continue providing 'old-style' spkgs or not, that is fine. But reading the relevant section in the developer guide hopefully should answer any questions you have about either type of spkg. The short version is that when you have a new version, you can just open a ticket on Trac to update the version, and since it is an optional spkg all we would have to do is make sure it doesn't break. Having a test suite we could run would help a lot with that - maybe it's already possible to do, and we just don't know how to run it properly? But hopefully it can actually be relatively painless, especially since you are just providing pure Python, right, not something that depends on compilers? Let us know if this answers any questions you have.

comment:14 follow-up: Changed 5 years ago by jhpalmieri

For what it's worth, I see the same error in Sage 6.1 and Sage 6.2, using version 0.1 of the spkg and the lines from 4:

┌────────────────────────────────────────────────────────────────────┐
│ Sage Version 6.1, Release Date: 2014-01-30                         │
│ Type "notebook()" for the browser-based notebook interface.        │
│ Type "help()" for help.                                            │
└────────────────────────────────────────────────────────────────────┘
sage: from ore_algebra.nullspace import *
sage: my_solver = cra(kronecker(gauss()))
sage: A = MatrixSpace(ZZ['x', 'y'], 4, 5).random_element()
sage: V = my_solver(A)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-268cdfff10da> in <module>()
----> 1 V = my_solver(A)

/Users/palmieri/Desktop/sage-6.1/local/lib/python2.7/site-packages/ore_algebra/nullspace.py in cra_solver(mat, degrees, infolevel)
   1275     def cra_solver(mat, degrees=[], infolevel=0) :
   1276         """See docstring of cra() for further information."""
-> 1277         return _cra(subsolver, max_modulus, proof, ncpus, mat, degrees, infolevel)
   1278     return cra_solver
   1279 

/Users/palmieri/Desktop/sage-6.1/local/lib/python2.7/site-packages/ore_algebra/nullspace.py in _cra(subsolver, max_modulus, proof, ncpus, mat, degrees, infolevel)
   1341             true_degrees = [max(max(e.degree() for e in v) for v in Vp)]
   1342         else:
-> 1343             true_degrees = [max(max(e.degree(x0) for e in v) for v in Vp) for x0 in x]
   1344 
   1345         if len(degrees) != len(x):

/Users/palmieri/Desktop/sage-6.1/local/lib/python2.7/site-packages/ore_algebra/nullspace.py in <genexpr>((v,))
   1341             true_degrees = [max(max(e.degree() for e in v) for v in Vp)]
   1342         else:
-> 1343             true_degrees = [max(max(e.degree(x0) for e in v) for v in Vp) for x0 in x]
   1344 
   1345         if len(degrees) != len(x):

/Users/palmieri/Desktop/sage-6.1/local/lib/python2.7/site-packages/ore_algebra/nullspace.py in <genexpr>((e,))
   1341             true_degrees = [max(max(e.degree() for e in v) for v in Vp)]
   1342         else:
-> 1343             true_degrees = [max(max(e.degree(x0) for e in v) for v in Vp) for x0 in x]
   1344 
   1345         if len(degrees) != len(x):

/Users/palmieri/Desktop/sage-6.1/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_element.pyc in degree(self, x)
    502             return self.element().degree(None)
    503         if not (isinstance(x, MPolynomial) and x.parent() is self.parent() and x.is_generator()):
--> 504             raise TypeError, "x must be one of the generators of the parent."
    505         return self.element().degree(x.element())
    506 

TypeError: x must be one of the generators of the parent.

So either I'm doing something wrong (in which case the example in nullspace.py is wrong, or at least unclear) or there is a bug.

comment:15 Changed 5 years ago by kcrisman

  • Authors changed from Karl-Dieter Crisman to Manuel Kauers
  • Reviewers changed from John Palmieri, Konstantin Ziegler to Karl-Dieter Crisman, John Palmieri, Konstantin Ziegler

So either I'm doing something wrong (in which case the example in nullspace.py is wrong, or at least unclear) or there is a bug.

And one that existed before, which would be a different ticket than this. Good to go?

comment:16 in reply to: ↑ 14 Changed 5 years ago by mkauers

Replying to jhpalmieri:

For what it's worth, I see the same error in Sage 6.1 and Sage 6.2, using version 0.1 of the spkg and the lines from 4: So either I'm doing something wrong (in which case the example in nullspace.py is wrong, or at least unclear) or there is a bug.

Thanks for insisting. Now I can reproduce it. When I tried it yesterday, something must have been different so that it went through without problem. The problem seems to be that the value of sage.ext.multi_modular.MAX_MODULUS has increased a few versions ago (perhaps between 5.9 and 6?), in combination with the following inconsistency:

sage: x, y = ZZ['x','y'].gens() sage: GF(1091)['x','y'].random_element().degree(x) ### works sage: GF(3037000453)['x','y'].random_element().degree(x) ### fails

Perhaps this should be fixed in the code of multivariate polynomial rings rather than by a workaround in nullspace.py?

@kcrisman: thanks for the pointer to the documentation. I will include self-testing routines in future versions.

comment:17 Changed 5 years ago by jhpalmieri

Okay, I think we can merge this. A test suite of some sort should be possible, and I'm glad you're going to work on it for a future release.

the following inconsistency:

    sage: x, y = ZZ['x','y'].gens()
    sage: GF(1091)['x','y'].random_element().degree(x) ### works
    sage: GF(3037000453)['x','y'].random_element().degree(x) ### fails

This is certainly inconsistent, but I'm not sure whether both should work or both should fail. Should x automatically be coerced to an element of the polynomial ring over GF(p)? I've posted a question on sage-devel, and eventually we should open a ticket about it.

comment:18 Changed 5 years ago by kcrisman

  • Status changed from needs_review to positive_review

comment:19 Changed 5 years ago by jhpalmieri

See #17366 for the multivariate polynomial ring issue.

comment:20 Changed 5 years ago by vbraun

Harald: I copied the spkg but I have no idea how to generate the txt metadata file, mirror-index.py doesn't do it.

comment:21 Changed 5 years ago by vbraun

  • Cc schilly added

comment:22 Changed 5 years ago by schilly

I'll take care of this, and this script has nothing to do with the packages.

comment:23 Changed 5 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.