sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

some small polybori interface fixes #7271

Closed malb closed 14 years ago

malb commented 14 years ago

CC: @burcin @sagetrac-PolyBoRi @sagetrac-drkirkby

Component: commutative algebra

Keywords: polybori

Author: Martin Albrecht

Reviewer: Mike Hansen

Merged: sage-4.3.1.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7271

mwhansen commented 14 years ago
comment:2

Why does variables() return an iterator? It returns a tuple for pretty much everything else in Sage. See #7077

malb commented 14 years ago
comment:3

Because that's what PolyBoRi expects internally.

mwhansen commented 14 years ago
comment:4

Can you explain in a bit more detail? How is PolyBoRi using that method?

malb commented 14 years ago
comment:5

Hi Mike, sorry for being so brief earlier, I was in a rush.

PolyBoRi calls this function from various functions which are called by the groebner_basis function. The ones I could find quickly are:

polybori-0.6/pyroot/polybori/rank.py:    return p.lex_lead().variables().next()
polybori-0.6/pyroot/polybori/ll.py:      return Monomial(v).variables().next().index()
polybori-0.6/testsuite/py/parsegat.py:    return p.lead().variables().next()

As you can see, it calls next() immediatly on the result of variables(). Right now, certain GB computations will fail with an AttributeError because of this.

malb commented 14 years ago
comment:6

I just received word that this will be changed in PolyBoRi.

1998e663-c3b5-4cf6-92c3-7f1771ca5185 commented 14 years ago
comment:7

http://bitbucket.org/brickenstein/polybori/changeset/48fab65072e9/

http://bitbucket.org/brickenstein/polybori/changeset/e238ae62b9e6/

Regarding parsegat, as you can see, I moved a corrected version between our repositories (similar one-liner). But there is no dependency on parsegat.py . The only funny thing about the recent versions of parsegat.py is that, you can see a poor mathetician trying to recognize patterns from bad encoded circuits. I still have nightmares from it.

malb commented 14 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,3 @@
 * implement var()
-* variables() is an iterator
+* implemented new functions required by PolyBoRi
+* fixed a few things in MPolynomialSystem
malb commented 14 years ago
comment:8

I prepared a new SPKG and a new patch.

The SPKG is available at:

http://sage.math.washington.edu/home/malb/spkgs/polybori-0.6.3.r1647-20091028.spkg

malb commented 14 years ago

Attachment: polybori_fixes.patch.gz

malb commented 14 years ago
comment:9

Mike, I reversed the iterator change in the latest patch. Can you review it?

malb commented 14 years ago
comment:10

I am attaching a new deps file to this ticket, to address

http://groups.google.com/group/sage-devel/browse_thread/thread/122f067d8947148d/

malb commented 14 years ago
comment:11

Attachment: deps.gz

The only thing changed in the deps file (which isn't under revision control) is that PolyBoRi now depends on M4RI

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:12

There are several issues I am aware of with m4ri, which should perhaps be sorted out before code is added that depends on it.

7171 - Although reported against HP-UX, this is more serious, as it is broken on Solaris too.

7037 - libm4ri thinks the C compiler is broken

I beleive the current version of PolyBoRi might actually build with Sun's compiler, but this would stop that, if it has a dependancy which does not.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:13

Oops, for some reason I was not aware of this ticket despite being CCed on it. Nor of #7375, which aims to address the issues in M4RI, so my comments are probably out of place. I will look at #7375 soon, but would be unable to review this ticket.

Dave

malb commented 14 years ago
comment:14

Hi, I was wondering if I someone could review this ticket now that the M4RI issues are resolved?

mwhansen commented 14 years ago
comment:15

The patch looks good. I'll add it first thing after 4.3, which should hopefully be out in the next day or two.

mwhansen commented 14 years ago

Reviewer: Mike Hansen

mwhansen commented 14 years ago

Merged: sage-4.3.1.alpha0