# Ticket #13760(closed defect: fixed)

Opened 6 months ago

## Wrong basic interval arithmetic in PolynomialRing

Reported by: Owned by: gmoroz AlexGhitza major sage-5.6 basic arithmetic interval, polynomial, multivariate burcin, mhansen, malb, zimmerma N/A Paul Zimmermann Guillaume Moroz sage-5.6.beta1

Some multiplications in multivariate polynomial rings over RIF or CIF are wrong:

```sage: R.<x,y> = PolynomialRing(RIF,2)
sage: RIF(-2,1)*x
0
```

## More tests

```sage: R.<x,y> = PolynomialRing(RIF,2)
sage: RIF(-2,1)          # OK
0.?e1
sage: RIF(-2,1)*x        # Wrong
0
sage: RIF(-2,1)*x == 0   # Wrong
True
sage: cmp(RIF(-2,1)*x,0) # Wrong
0
sage: RIF(2,5)*x         # OK
1.?e1*x
sage: RIF(2,5)*x == 0    # OK
False
```

## Code digging

The problem comes from the coercion:

```sage: R(RIF(-2,1))
0
sage: R(RIF(-2,1)) == 0
True
```

This in turn comes from the creation (in __init__ of class MPolynomial_polydict in sage.rings.polynomial.multi_polynomial_element) of a PolyDict? object (defined in sage.rings.polynomial.polydict) with the option remove_zero == True.

```from sage.rings.polynomial.polydict import PolyDict
sage: PolyDict({(0,0):1}, remove_zero=True) # OK
PolyDict with representation {(0, 0): 1}
sage: PolyDict({(0,0):0}, remove_zero=True) # OK
PolyDict with representation {}
sage: PolyDict({(0,0):RIF(-1,1)}, remove_zero=True) # Wrong
PolyDict with representation {}
```

To check if x is different from 0, PolyDict? uses the test x != 0, which actually checks for disjointness in interval field:

```sage: RIF(-2,1) != 0
False
```

## Possible correction

This bug might be corrected by replacing in PolyDict? the tests x != zero by one of:

• cmp(x,zero) != 0
• not x.is_zero()
• not x == zero

Apply:

## Change History

### comment:1 Changed 6 months ago by gmoroz

• Description modified (diff)

### Changed 6 months ago by gmoroz

A patch changing x != zero by not x == zero.

### comment:2 Changed 6 months ago by gmoroz

• Description modified (diff)
• Authors set to gmoroz

### comment:3 Changed 6 months ago by gmoroz

• Status changed from new to needs_review

### comment:5 Changed 6 months ago by zimmerma

Guillaume, a non-regression test is missing (also for #7712).

Paul

### comment:6 Changed 6 months ago by zimmerma

• Status changed from needs_review to needs_work
• Work issues set to add non-regression test

### comment:7 Changed 5 months ago by zimmerma

all tests pass on top of 5.4.1, just waiting for a non-regression test.

Paul

### Changed 5 months ago by gmoroz

add regression tests (to be applied after the fix patch)

### comment:8 Changed 5 months ago by zimmerma

Guillaume, why didn't you add as regression tests the examples in the description of #7712 and this ticket?

Paul

### comment:9 Changed 5 months ago by zimmerma

all tests pass with the regression tests.

Paul

### comment:10 Changed 5 months ago by gmoroz

I attached the patch with regression tests with user level examples on #7712 (this choice was quite random).

### comment:11 Changed 5 months ago by gmoroz

• Status changed from needs_work to needs_review

### Changed 5 months ago by gmoroz

Includes regression tests for #7712 and #13760 (same patch as in #7712, moved here for convenience)

### comment:12 Changed 5 months ago by zimmerma

• Status changed from needs_review to positive_review
• Reviewers set to Paul Zimmermann
• Work issues add non-regression test deleted
• Authors changed from gmoroz to Guillaume Moroz

### comment:13 follow-up: ↓ 15 Changed 5 months ago by jdemeyer

• Milestone changed from sage-5.5 to sage-5.6

Please indicate which patch(es) need to be applied.

### comment:14 Changed 5 months ago by zimmerma

• Description modified (diff)

### comment:15 in reply to: ↑ 13 Changed 5 months ago by zimmerma

Please indicate which patch(es) need to be applied.

Done. Paul

### comment:16 follow-up: ↓ 17 Changed 5 months ago by tscrim

One very minor documentation thing, there should be a blankline after any double colon :: (see line 514 in the trac_13760_regression_tests.patch ). Thanks.

### comment:17 in reply to: ↑ 16 ; follow-up: ↓ 18 Changed 5 months ago by gmoroz

One very minor documentation thing, there should be a blankline after any double colon :: (see line 514 in the trac_13760_regression_tests.patch ). Thanks.

It seems that I cannot attach a new patch for that. Is it because of the positive review status?

### comment:18 in reply to: ↑ 17 ; follow-up: ↓ 20 Changed 5 months ago by tscrim

• Status changed from positive_review to needs_work
• Work issues set to docstrings

It seems that I cannot attach a new patch for that. Is it because of the positive review status?

Strange...I don't think so, but I'll set this to needs work and try it then.

### comment:19 Changed 5 months ago by gmoroz

• Description modified (diff)

### comment:20 in reply to: ↑ 18 Changed 5 months ago by gmoroz

Strange...I don't think so, but I'll set this to needs work and try it then.

My bad, it appears that my file didn't have permissions to be read.

### comment:21 Changed 5 months ago by tscrim

• Status changed from needs_work to positive_review
• Description modified (diff)

Thanks for fixing this.

### comment:22 Changed 5 months ago by jdemeyer

• Work issues docstrings deleted

### comment:23 Changed 5 months ago by jdemeyer

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