Opened 7 years ago

Closed 7 years ago

# Convert factorization to symbolic expression

Reported by: Owned by: gagern major sage-6.10 symbolics Ralf Stephan Jeroen Demeyer N/A 9c461e3 9c461e3f14d5d48fffc2bb59ee17e37a8afff125

### Description

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.

### comment:1 Changed 7 years ago by rws

• Branch set to u/rws/coerce_factorization_of_polynomial_to_symbolic_expression

### comment:2 Changed 7 years ago by rws

• Authors set to Ralf Stephan
• Commit set to ec9cd10617f78bb1a8a0d1550d1accb6c765bc1e
• Milestone changed from sage-6.5 to sage-6.10
• Status changed from new to needs_review

New commits:

 ​ec9cd10 `17624: coerce factorizations to SR`

### comment:3 Changed 7 years ago by bruno

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
```

### comment:4 Changed 7 years ago by rws

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.

### comment:5 Changed 7 years ago by rws

• Status changed from needs_review to needs_work

OTOH I just found a bug (77?).

### comment:6 Changed 7 years ago by git

• Commit changed from ec9cd10617f78bb1a8a0d1550d1accb6c765bc1e to d9f2c454bab76af892452cfefbf4b62790feb6c5

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

 ​d9f2c45 `17624: fix previous commit`

### comment:7 Changed 7 years ago by rws

• Status changed from needs_work to needs_review

### comment:8 follow-up: ↓ 16 Changed 7 years ago by nbruin

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.

### comment:9 Changed 7 years ago by jdemeyer

• Status changed from needs_review to needs_work
• Summary changed from Coerce factorization of polynomial to symbolic expression to Convert factorization to symbolic expression

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

### comment:10 Changed 7 years ago by git

• Commit changed from d9f2c454bab76af892452cfefbf4b62790feb6c5 to 23b24efe0f7355f312b5b38f9cf8d520240903a3

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

 ​23b24ef `17624: convert factorization unit to SR`

### comment:11 follow-up: ↓ 12 Changed 7 years ago by rws

• Status changed from needs_work to needs_review

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

### comment:12 in reply to: ↑ 11 Changed 7 years ago by jdemeyer

• Status changed from needs_review to needs_work

Replying to rws:

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()))
```

### comment:13 Changed 7 years ago by git

• Commit changed from 23b24efe0f7355f312b5b38f9cf8d520240903a3 to 9d1432aa35cf5005a6431c0911b2e96b0de615f7

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

 ​9d1432a `17426: cosmetics`

### comment:14 Changed 7 years ago by rws

• Status changed from needs_work to needs_review

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

### comment:15 Changed 7 years ago by git

• Commit changed from 9d1432aa35cf5005a6431c0911b2e96b0de615f7 to 9c461e3f14d5d48fffc2bb59ee17e37a8afff125

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

 ​9c461e3 `17426: fix typo`

### comment:16 in reply to: ↑ 8 ; follow-up: ↓ 17 Changed 7 years ago by mmezzarobba

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()?

### comment:17 in reply to: ↑ 16 ; follow-up: ↓ 18 Changed 7 years ago by jdemeyer

Replying to mmezzarobba:

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.

### comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 7 years ago by mmezzarobba

Replying to jdemeyer:

Replying to mmezzarobba:

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.

### comment:19 in reply to: ↑ 18 Changed 7 years ago by jdemeyer

Replying to mmezzarobba:

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__`.

### comment:20 Changed 7 years ago by jdemeyer

• Reviewers set to Jeroen Demeyer
• Status changed from needs_review to positive_review

### comment:21 Changed 7 years ago by vbraun

• Branch changed from u/rws/coerce_factorization_of_polynomial_to_symbolic_expression to 9c461e3f14d5d48fffc2bb59ee17e37a8afff125
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.