Opened 3 years ago

Closed 3 years ago

#27421 closed enhancement (fixed)

.coefficient of multi-variate polynomial should accept output of .exponents()

Reported by: dkrenn Owned by:
Priority: minor Milestone: sage-8.7
Component: algebra Keywords: easy, beginner
Cc: gh-ChamanAgrawal Merged in:
Authors: Chaman Agrawal Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f5e3eb4 (Commits, GitHub, GitLab) Commit: f5e3eb47b1f08fafcc9578327eab74c84be8491c
Dependencies: Stopgaps:

Status badges

Description

sage: P.<x,y> = QQ[]
sage: x.coefficient(x.exponents()[0])

gives

TypeError: The input degrees must be a dictionary of variables to exponents.

and one has to use

sage: x.coefficient(list(x.exponents()[0]))
1

as a workaround. However, it seems strange, that the exponents output by the same object are not accepted as input to another method.

Change History (13)

comment:1 Changed 3 years ago by gh-ChamanAgrawal

  • Authors set to Chaman Agrawal
  • Branch set to u/gh-ChamanAgrawal/27421_coefficient
  • Cc gh-ChamanAgrawal added
  • Commit set to c5c4e8185d3c266e549e7b438f53417095e37330
  • Status changed from new to needs_review

New commits:

c5c4e81Handle ETuple in coefficient()

comment:2 Changed 3 years ago by gh-ChamanAgrawal

This is my first time in sagemath so sorry if I have missed some important community standards.

For the issue I thought two possible solutions:

1) Change the return type of exponents() to list.

2) Change coefficient() to handle the return value of exponents()

Changing the return type of exponents() will affect all the dependent functions of exponents() so I have changed coefficient() to handle ETuple also ( return type of exponents() ).

comment:3 Changed 3 years ago by git

  • Commit changed from c5c4e8185d3c266e549e7b438f53417095e37330 to a406d1ec194bac92656be1c450234b4bf2a20b92

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

a406d1eAdded Doctest for #27421

comment:4 Changed 3 years ago by tscrim

if degrees_list is None:

is vacuous because list(foo) should never return None. Also, it seems wasteful to convert it to a list. Why not just use the ETuple directly with

exps[i] = int((<ETuple>degrees).get_exp(i))

Also, we want to move away from the old-style Cython declarations

-for i from 0<=i<gens:
+for i in range(gens):

comment:5 Changed 3 years ago by git

  • Commit changed from a406d1ec194bac92656be1c450234b4bf2a20b92 to 5c5fe615439bc6dbb8f45250dace2074224e1681

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

5c5fe61Using ETuple directly instead of converting to list

comment:6 Changed 3 years ago by tscrim

Last two little things:

1 - You should test f.coefficient(x.exponents()[0]) to show that the output matches the above (i.e., it is an equivalent input). 2 - Your line exps[i] = int((<ETuple>degrees).get_exp(i)) is over-indented.

Thanks.

comment:7 Changed 3 years ago by git

  • Commit changed from 5c5fe615439bc6dbb8f45250dace2074224e1681 to db2f0458b3e6b96b6d8ce032bb1d106256657dc4

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

db2f045Removed over indentation and added equivalence test

comment:8 follow-up: Changed 3 years ago by tscrim

No, that is not what I wanted for the test. I was looking for this change

-            sage: x.coefficient(x.exponents()[0])
-            1
+            sage: f.coefficient(x.exponents()[0])
+            y^2 + y + 1

comment:9 in reply to: ↑ 8 Changed 3 years ago by gh-ChamanAgrawal

Replying to tscrim:

No, that is not what I wanted for the test. I was looking for this change

-            sage: x.coefficient(x.exponents()[0])
-            1
+            sage: f.coefficient(x.exponents()[0])
+            y^2 + y + 1

But that is not how exponents() works. In the Multivariate Polynomial Ring in x, y over Rational Field R.<x,y> = QQ[] x has exponent (1,0) so x.exponents() returns [(1, 0)] and so f.coefficient(x.exponents()[0]) returns 1.

I don't think changing the behavior of exponents() is the right course of action, So can you please explain further the desired behaviour of the coefficient() in this case.

comment:10 Changed 3 years ago by tscrim

Ah, right, it has the y value explicitly set. Okay, then let's change the second part of your test to make it more clear what is going on, with a bit more explanation:

            sage: f.coefficient(x)
            y^2 + y + 1

        Note that exponents have all variables specified::

            sage: x.coefficient(x.exponents()[0])
            1
            sage: f.coefficient([1,0])
            1
            sage: f.coefficient({x:1,y:0})
            1

comment:11 Changed 3 years ago by git

  • Commit changed from db2f0458b3e6b96b6d8ce032bb1d106256657dc4 to f5e3eb47b1f08fafcc9578327eab74c84be8491c

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

f5e3eb4Modified DocTests

comment:12 Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thanks. LGTM.

comment:13 Changed 3 years ago by vbraun

  • Branch changed from u/gh-ChamanAgrawal/27421_coefficient to f5e3eb47b1f08fafcc9578327eab74c84be8491c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.