Opened 7 years ago
Closed 7 years ago
#17624 closed enhancement (fixed)
Convert factorization to symbolic expression
Reported by: | gagern | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.10 |
Component: | symbolics | Keywords: | |
Cc: | Merged in: | ||
Authors: | Ralf Stephan | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | 9c461e3 (Commits, GitHub, GitLab) | 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^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.
Change History (21)
comment:1 Changed 7 years ago by
- Branch set to u/rws/coerce_factorization_of_polynomial_to_symbolic_expression
comment:2 Changed 7 years ago by
- Commit set to ec9cd10617f78bb1a8a0d1550d1accb6c765bc1e
- Milestone changed from sage-6.5 to sage-6.10
- Status changed from new to needs_review
comment:3 Changed 7 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^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
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
- Status changed from needs_review to needs_work
OTOH I just found a bug (77?).
comment:6 Changed 7 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 7 years ago by
- Status changed from needs_work to needs_review
comment:8 follow-up: ↓ 16 Changed 7 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 7 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 7 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 follow-up: ↓ 12 Changed 7 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 7 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 7 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 7 years ago by
- Status changed from needs_work to needs_review
So, assuming SR.unit==1
is safe then...
comment:15 Changed 7 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 ; follow-up: ↓ 17 Changed 7 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 ; follow-up: ↓ 18 Changed 7 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 ; follow-up: ↓ 19 Changed 7 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 7 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 7 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:21 Changed 7 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