Opened 13 years ago

Closed 11 years ago

# Polynomial.apply_map()

Reported by: Owned by: Marc Mezzarobba Alex Ghitza minor sage-5.0 algebra polynomial, map Daniel Krenn sage-5.0.beta6 Marc Mezzarobba, Julian Rueth Daniel Krenn N/A #11981

Computing, for instance, the complex conjugate of a polynomial currently requires going through its list or dictionary representation. Polynomials could provide a method similar to Matrix.apply_map() to make this kind of computations easier.

Apply

to the sage repository.

### comment:1 Changed 13 years ago by Marc Mezzarobba

Status: new → needs_review

The attached patch adds naive implementations of apply_map() and of a second similar method, map_coefficients(), modeled after that of multivariate polynomials.

Note that I have as good as no experience with Python or Sage development—so sorry for any newbie errors... and please review carefully! :-)

### comment:2 Changed 13 years ago by Jason Grout

One suggestion---the names of the new functions do not seem to indicate to me that they only operate on the nonzero coefficients, so it would be hard for me to remember what the difference between map_coefficients and apply_map is.

p.apply_map(nonzero_only=True)

or something like that?

### comment:3 Changed 13 years ago by Marc Mezzarobba

The idea was to stay compatible with MPolynomial.map_coefficients(), which operates on nonzero coefficients. Perhaps we could remove apply_map() rather that map_coefficients(), and add an option to make map_coefficients() map over zero coefficients too. (Ignoring them by default seems sensible, since (i) there is really no mathematical difference between zeros below and above the leading coefficient, and (ii) the functions one will typically pass to map_coefficients() are ring homomorphisms.)

What do you think?

### comment:4 Changed 13 years ago by Jason Grout

That sounds cleaner to me. I'm likely not to use this function very much, so I shouldn't be the last authority on it, though.

### comment:5 Changed 12 years ago by David Loeffler

Status: needs_review → needs_work

Any progress here? I agree with Jason's contention that having both apply_map and map_coefficients is less than ideal, so I suggest that we put this back to "needs work" pending a patch that implements this.

### comment:6 Changed 11 years ago by Daniel Krenn

I would suggest to map only non-zero coefficients because of the following reason: When mapping all (really all) zeros to a non-zero, the result is not a polynomial any more, so one would have to restrict that. But how should this restriction look like? A good restriction strategy (strategy which coefficients to choose and apply the map on) should be generalizable to the multivariate case. E.g. restriction by degree (i.e. only change the coefficients with indizes 0 to degree) could work in the univariate case, but does not work for multivariate polynomials.

If one really wants to map 0 to a non-zero, then this should be done somewhere else, i.e., not in map_coefficients

### comment:7 Changed 11 years ago by Julian Rüth

Authors: → Marc Mezzarobba, Julian Rueth sage-wishlist → sage-5.0 needs_work → needs_review

I agree that mapping 0 to non-zero should not be a part of map_coefficients().

I rewrote the patch to only contain map_coefficients() for polynomial elements.

Was there any reason for distinguishing between polynomial_element and polynomial_element_generic? I removed the distinction and it turned out that this implementation is even faster for the example polynomials.

### comment:8 Changed 11 years ago by Julian Rüth

Description: modified (diff)

apply trac_8718.patch

### comment:9 follow-up:  10 Changed 11 years ago by Marc Mezzarobba

Thanks for taking care of that! (And shame on me that I didn't!)

As for the distinction between polynomial_element and polynomial_element_generic, it may indeed have been a speed issue, but I can't remember the details. Anyway, if the current version does the job, I'm fine with it.

[I'm not sure what the rules are here: am I allowed to review the new version of the patch though being its initial author?]

### comment:10 in reply to:  9 Changed 11 years ago by Julian Rüth

[I'm not sure what the rules are here: am I allowed to review the new version of the patch though being its initial author?]

Yes, in my experience this is ok and actually happens frequently.

### comment:11 Changed 11 years ago by Daniel Krenn

Status: needs_review → needs_work

In #11981 the map_coefficients of a multivariate polynomial was adapted to change the base ring of the coefficients. I think we should also do this here for this function, since the two should have the same behavior.

### comment:12 follow-up:  13 Changed 11 years ago by Julian Rüth

Status: needs_work → needs_info

True. I wanted to push this to a later ticket but we can also talk about this here.

I wonder if this "new base ring" should default to the codomain of f (if the function is actually a sage 'Map'). For me this has always been what I wanted. Any opinions on that?

### comment:13 in reply to:  12 Changed 11 years ago by Daniel Krenn

Work issues: → check if function knows its codomain and use it

I wonder if this "new base ring" should default to the codomain of f (if the function is actually a sage 'Map'). For me this has always been what I wanted. Any opinions on that?

That is a good idea, we should implement that. So the strategy would be the following: If a new base ring is given, then we use it, otherwise, we check if the function knowns its codomain and if yes use it, otherwise we do not change the base ring.

### Changed 11 years ago by Julian Rüth

consider the codomain in map_coefficients for univariate polynomials

### Changed 11 years ago by Julian Rüth

consider the codomain in map_coefficients for multivariate polynomials

### comment:15 Changed 11 years ago by Julian Rüth

Description: modified (diff) needs_info → needs_review

### comment:16 Changed 11 years ago by Julian Rüth

Work issues: check if function knows its codomain and use it

### comment:17 Changed 11 years ago by Daniel Krenn

Dependencies: → #11981

### comment:18 Changed 11 years ago by Daniel Krenn

Cc: Daniel Krenn added → Daniel Krenn needs_review → positive_review

### comment:19 Changed 11 years ago by Jeroen Demeyer

Merged in: → sage-5.0.beta6 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.