Convert factorization to symbolic expression

[gagern]

Dealing with polynomial rings is nice, but the fact that they always present their content in fully expanded form can make it hard to read results. For a single polynomial, computing its factorization can help. But for e.g. a vector or matrix composed of factorizations, this is not possible.

sage: R.<a,b> = QQ[]
sage: m = matrix([[a^2-b^2, a^3-a*b^2], [a*b + b^2, -77*a+77*b]])
sage: factor(m[0,0])
(a - b) * (a + b)
sage: m.apply_map(factor)
TypeError: x must be a list
sage: SR(factor(m[0,0]))
TypeError: 
sage: m.apply_map(lambda x: SR(str(factor(x))))
[  (a + b)*(a - b) (a + b)*(a - b)*a]
[        (a + b)*b      -77*a + 77*b]
sage: m[1,1].factor()
(-77) * (a - b)

It would be nice if the apply_map(factor) call would simply work, returning a matrix of symbolic expressions to help reading that matrix. The detour via str is a very hackish workaround, in my opinion.

Note that SR will automatically expand some things, as evidenced by the coeffcicient 77 in the example above. I guess that could only be avoided if we could have matrices of factorizations. That would be a viable alternative, and might be worth its own ticket, but I think coercion to SR might have other benefits as well, so this ticket here is about that coercion.

Component: symbolics

Author: Ralf Stephan

Branch/Commit: 9c461e3

Reviewer: Jeroen Demeyer

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

[rwst]

Branch: u/rws/coerce_factorization_of_polynomial_to_symbolic_expression

[rwst]

Commit: ec9cd10

[rwst]

Author: Ralf Stephan

[rwst]

New commits:

ec9cd10

17624: coerce factorizations to SR

[bgrenet]

comment:3

While the coercion seems to work as expected, it does not solve the "apply_map" issue. What else is needed for this?

sage: R.<a,b> = QQ[]
sage: m = matrix([[a^2-b^2, a^3-a*b^2], [a*b + b^2, -77*a+77*b]])
sage: m.apply_map(factor)
Traceback (most recent call last):
...
TypeError: x must be a list

[rwst]

comment:4

Well it doesn't work with integers, either:

sage: m = matrix([[6,15], [35,49]])
sage: m.apply_map(factor)
...

and so is clearly not a problem of this ticket.

Of course you need to explicitly cast to SR (like you did in the ticket description):

sage: R.<a,b> = QQ[]
sage: m = matrix([[a^2-b^2, a^3-a*b^2], [a*b + b^2, -77*a+77*b]])
sage: f = lambda n: SR(factor(n))
sage: m.apply_map(f)
[   (a + b)*(a - b) -(a + b)*(a - b)*a]
[         (a + b)*b              a - b]

It's just that the string crutch is no longer needed.

[rwst]

comment:5

OTOH I just found a bug (77?).

[sagetrac-git]

Changed commit from ec9cd10 to d9f2c45

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d9f2c45

17624: fix previous commit

[nbruin]

comment:8

It seems that the following, using SR's "factor", works pretty well for the problem in the question:

sage: factor(SR(m))
[  (a + b)*(a - b) (a + b)*(a - b)*a]
[        (a + b)*b      -77*a + 77*b]

The problem with doing it the other way around is that the intermediate result would have to be a matrix with elements coming from ... Factorization? While we could have that, it would require a significant design and refactoring effort. Do we have a good reason to do that?

With factorizations themselves convertible to SR (which is quite reasonable and easy to do, as rws shows) one can do:

matrix(SR,m.nrows(),m.ncols(),[SR(factor(c)) for c in m.list()])

which is not hacky, albeit a bit verbose. It's basically "apply_map" spelled out.

[jdemeyer]

comment:9

I think it's safer to explicitly convert the unit also to SR.

[sagetrac-git]

Changed commit from d9f2c45 to 23b24ef

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

23b24ef

17624: convert factorization unit to SR

[rwst]

comment:11

I thought that would be done by giving it second for multiplication?

[jdemeyer]

comment:12

Replying to @rwst:

I thought that would be done by giving it second for multiplication?

Well, I think it's better to be explicit. But your comment inspired me that you should do this instead:

return prod([SR(p)**e for p,e in x], SR(x.unit()))

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

9d1432a

17426: cosmetics

[sagetrac-git]

Changed commit from 23b24ef to 9d1432a

[rwst]

comment:14

So, assuming SR.unit==1 is safe then...

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

9c461e3

17426: fix typo

[sagetrac-git]

Changed commit from 9d1432a to 9c461e3

[mezzarobba]

comment:16

Replying to @nbruin:

The problem with doing it the other way around is that the intermediate result would have to be a matrix with elements coming from ... Factorization? While we could have that, it would require a significant design and refactoring effort.

Is there any real obstacle to making Factorizations Elements of their universe()?

[jdemeyer]

comment:17

Replying to @mezzarobba:

Is there any real obstacle to making Factorizations Elements of their universe()?

But that parent couldn't possibly be a ring (what's the sum of 2 factorisations?), so you wouldn't be able to make matrices over it.

[mezzarobba]

comment:18

Replying to @jdemeyer:

Replying to @mezzarobba:

Is there any real obstacle to making Factorizations Elements of their universe()?

But that parent couldn't possibly be a ring (what's the sum of 2 factorisations?), so you wouldn't be able to make matrices over it.

When the universe is a ring, + could always return a “normal” element. In other words: what I'm suggesting is not to have “Factorizations over Foo” as a new parent, but to make Factorizations over Foo (an alternative representation of) elements of Foo.

[jdemeyer]

comment:19

Replying to @mezzarobba:

make Factorizations over Foo (an alternative representation of) elements of Foo.

I don't think that this will work properly. Within the current Parent/Element framework, it would require that FactorizationOfFoo is a subclass of Foo. Since Foo is usually a Cython class, it must be the first base class. Unless you want to play metaclass games, this is the wrong __mro__.

[jdemeyer]

Reviewer: Jeroen Demeyer

[vbraun]

Changed branch from u/rws/coerce_factorization_of_polynomial_to_symbolic_expression to 9c461e3