Opened 11 years ago

Closed 11 years ago

is_monomial and is_term

Reported by: Owned by: was AlexGhitza minor sage-4.7.2 algebra sd32 sage-4.7.2.alpha3 William Stein Mike Hansen N/A

Ut oh, the word "monomial" turns out to be ambiguous! There are two definitions!:

And in Sage we evidently have *randomly* (?) and inconsistenly chosen between the two definitions, which is very unfortunate.

```sage: R.<x> =QQ[]
sage: (2*x).is_monomial()     # definition 2 in univariate poly
True
sage: (x).is_monomial()
True
sage: R.<x,y> =QQ[]
sage: (2*x).is_monomial()     # definition 1 in multivariate poly
False
sage: x.is_monomial()
True
```

Etc. Fortunately, `is_monomial()` is called in only about 5 or 6 places in the entire Sage source library, according to `search_src('is_monomial')`.

Reading the argument in Wikipedia further, and discussing this with Tom Boothby, we've decided the following would work for us.

1. Introduce a new method `is_term`, which returns True for `a*x^i*y^j...`, i.e. it allows a coefficient. Fortunately, `is_term` is currently used nowhere in Sage.
1. Unify `is_monomial` to require the coefficient to be 1. This means changing univariate polynomials to be consistent with multivariate polynomials.

Apply trac_11747.patch to the Sage library.

comment:1 Changed 11 years ago by was

• Status changed from new to needs_review

comment:2 Changed 11 years ago by mhansen

Luckily, this choice is consistent with all of the code in sage/combinat/free_module.py .

comment:3 Changed 11 years ago by mhansen

• Status changed from needs_review to positive_review

Looks good to me.

comment:4 Changed 11 years ago by leif

• Authors set to William Stein
• Description modified (diff)
• Reviewers set to Mike Hansen

comment:5 follow-up: ↓ 7 Changed 11 years ago by leif

Any clue what can cause the following?

```sage -t -long -force_lib "devel/sage/sage/rings/polynomial/multi_polynomial_element.py"
**********************************************************************
File "/tmp/Sage/release/sage-4.7.2.alpha2/devel/sage/sage/rings/polynomial/multi_polynomial_element.py", line 101:
sage: x + QQbar.random_element() # indirect doctest
Expected:
x - 2
Got:
x + 4
**********************************************************************
1 of   5 in __main__.example_2
***Test Failed*** 1 failures.
```

This doesn't happen with only this patch applied, and no other patch (merged so far) touches `sage/rings/polynomial/multi_polynomial_element.py` (nor `qqbar.py`).

No single of the other patches merged with this one seems to cause this doctest error alone, i.e., apparently only a combination.

(And it's not #10635.)

comment:6 Changed 11 years ago by fbissey

One thing that strikes me is that the call is for QQbar.random_element(). It seems to me that unless there is a good reason for it, the fact that we always had the same result regardless of architecture is just blind luck.

So was there ever a justification why it should be "-2"?

comment:7 in reply to: ↑ 5 Changed 11 years ago by leif

Any clue what can cause the following? [...]

Oh, my bad; must have been blind or confused something when trying to track down another error.

The doctest failure most probably originates from the patch at #11725, so this ticket is certainly the wrong one to report on.

Sorry for the noise.

comment:8 Changed 11 years ago by leif

• Merged in set to sage-4.7.2.alpha3
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.