Opened 4 years ago
Closed 3 years ago
#17624 closed enhancement (fixed)
Convert factorization to symbolic expression
Reported by:  gagern  Owned by:  

Priority:  major  Milestone:  sage6.10 
Component:  symbolics  Keywords:  
Cc:  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  9c461e3 (Commits)  Commit:  9c461e3f14d5d48fffc2bb59ee17e37a8afff125 
Dependencies:  Stopgaps: 
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^2b^2, a^3a*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.
Change History (21)
comment:1 Changed 3 years ago by
 Branch set to u/rws/coerce_factorization_of_polynomial_to_symbolic_expression
comment:2 Changed 3 years ago by
 Commit set to ec9cd10617f78bb1a8a0d1550d1accb6c765bc1e
 Milestone changed from sage6.5 to sage6.10
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
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^2b^2, a^3a*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 3 years ago by
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^2b^2, a^3a*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 3 years ago by
 Status changed from needs_review to needs_work
OTOH I just found a bug (77?).
comment:6 Changed 3 years ago by
 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 3 years ago by
 Status changed from needs_work to needs_review
comment:8 followup: ↓ 16 Changed 3 years ago by
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 3 years ago by
 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 3 years ago by
 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 followup: ↓ 12 Changed 3 years ago by
 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 3 years ago by
 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 3 years ago by
 Commit changed from 23b24efe0f7355f312b5b38f9cf8d520240903a3 to 9d1432aa35cf5005a6431c0911b2e96b0de615f7
Branch pushed to git repo; I updated commit sha1. New commits:
9d1432a  17426: cosmetics

comment:14 Changed 3 years ago by
 Status changed from needs_work to needs_review
So, assuming SR.unit==1
is safe then...
comment:15 Changed 3 years ago by
 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 ; followup: ↓ 17 Changed 3 years ago by
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 ; followup: ↓ 18 Changed 3 years ago by
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 ; followup: ↓ 19 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:21 Changed 3 years ago by
 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
New commits:
17624: coerce factorizations to SR