factoring rational functions -- inconsistent error messages!

[John Cremona]

I define a rational function in two variables over a finite field:

{{{
sage: R.<x,y> = GF(2)[]
sage: f = x*y/(x+y)
sage: f.parent()
Fraction Field of Multivariate Polynomial Ring in x, y over Finite
Field of size 2
}}}

I try to factor it, and get this error:

{{{
sage: f.factor()
---------------------------------------------------------------------------
NotImplementedError Traceback (most recent call last)

/home/masgaj/.sage/temp/host_56_150/17587/_home_masgaj__sage_init_sage_0.py
in <module>()

/local/jec/sage-4.3.rc0/local/lib/python2.6/site-packages/sage/rings/fraction_field_element.so
in sage.rings.fraction_field_element.FractionFieldElement.factor
(sage/rings/fraction_field_element.c:2972)()

/local/jec/sage-4.3.rc0/local/lib/python2.6/site-packages/sage/rings/polynomial/multi_polynomial_libsingular.so
in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular.factor
(sage/rings/polynomial/multi_polynomial_libsingular.cpp:22701)()

NotImplementedError: proof = True factorization not implemented. Call
factor with proof=False.
}}}

So I do what I am told, but:

{{{
sage: f.factor(proof=False)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)

/home/masgaj/.sage/temp/host_56_150/17587/_home_masgaj__sage_init_sage_0.py
in <module>()

TypeError: factor() takes no keyword arguments
}}}

Worth a ticket, I think?

Also, does anyone know what the proof parameter for this sort of
factorization actually means?

John

[Sebastian Pancratz]

Dear John,

I haven't written any of the code involved, so I am not absolutely
sure about this, but after looking at the code for a few minutes, I
think the following is the case:

The "FractionFieldElement" objects have a method "factor", but this
takes no arguments other than "self". This explains the error message
in the second case you describe. On the other hand, the
"MPolynomial_libsingular" objects provide the method "factor(self,
proof=True)". Here, however, the parameter "proof" is only used to
prevent certain inputs (with the option "proof=True" set) from being
passed to Singular, presumably(?) in cases when Singular's results
might not be reliable. There appears to be no further use of the
parameter.

I think this is something that ought to fixed. (N.B. I don't think I
know enough about what kind of factoring methods rings in Sage
typically offer.) If rings often offer further arguments to the
method "factor", then I think the method "factor" for
FractionFieldElement objects should be modified to allow the same
arguments. On the other hand, if "MPolynomial_libsingular" is the
only case where "factor" takes further arguments, perhaps the second
argument ought to be removed?

Sebastian

[Sebastian Pancratz]

Further to my earlier post, a search for "def factor(self," in the
Sage library reveals that there are a lot of instances where the
factoring method supports various arguments. Thus, at the very least,
I think the fraction field implementation should allow further
arguments, too, and simply forward them, that is, the code should
change from

def factor(self):
return self.numerator().factor()/self.denominator().factor()

to

def factor(self, *args)
return self.numerator().factor(args)/self.denominator().factor
(args)

Moreover, I guess the parameter "proof" should be documented in the
method "factor" for multivariate polynomials.

Sebastian

[John Cremona]

2010/1/7 Sebastian Pancratz <sa...@pancratz.org>:

> Further to my earlier post, a search for "def factor(self," in the
> Sage library reveals that there are a lot of instances where the
> factoring method supports various arguments.  Thus, at the very least,
> I think the fraction field implementation should allow further
> arguments, too, and simply forward them, that is, the code should
> change from
>
> def factor(self):
>    return self.numerator().factor()/self.denominator().factor()
>
> to
>
> def factor(self, *args)
>    return self.numerator().factor(args)/self.denominator().factor
> (args)
>

That looks like a good, simple solution. Can anyone see any
drawbacks? If not, it would be a simple patch (but requiring full
testing!)

John

> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org
>
>

[Jason Grout]

Sebastian Pancratz wrote:
> Further to my earlier post, a search for "def factor(self," in the
> Sage library reveals that there are a lot of instances where the
> factoring method supports various arguments. Thus, at the very least,
> I think the fraction field implementation should allow further
> arguments, too, and simply forward them, that is, the code should
> change from
>
> def factor(self):
> return self.numerator().factor()/self.denominator().factor()
>
> to
>
> def factor(self, *args)
> return self.numerator().factor(args)/self.denominator().factor
> (args)
>
> Moreover, I guess the parameter "proof" should be documented in the
> method "factor" for multivariate polynomials.

I haven't read this thread very carefully, but just from a python
perspective, you probably want something like the following to pass on
all of the arguments.

def factor(self, *args, **kwds)
return self.numerator().factor(*args,**kwds)/self.denominator().factor
(*args,**kwds)

Thanks,

Jason

[John Cremona]

2010/1/7 Jason Grout <jason...@creativetrax.com>:

> I haven't read this thread very carefully, but just from a python
> perspective, you probably want something like the following to pass on all
> of the arguments.
>
> def factor(self, *args, **kwds)
>     return self.numerator().factor(*args,**kwds)/self.denominator().factor
> (*args,**kwds)
>

Thanks, Jason -- very helpful.

John

[Sebastian Pancratz]

In fact, this is not quite what I wanted. I should have written

def factor(self, *args, **kwds)
return self.numerator().factor(*args, **kwds) / \
self.denominator().factor(*args, **kwds)

In the meantime, while waiting for further comments, I've put this up
as ticket #7868. I'll test this now and, if nothing fails, upload the
patch later today.

Sebastian

On Jan 7, 4:50 pm, John Cremona <john.crem...@gmail.com> wrote:
> 2010/1/7 Sebastian Pancratz <s...@pancratz.org>:

[Sebastian Pancratz]

Jason, you beat me to it. Thank you for letting me know, though!

Sebastian