Opened 4 years ago

Closed 4 years ago

Disallow positive characteristic in the symbolic ring

Reported by: Owned by: bruno critical sage-8.1 commutative algebra polynomial, symbolics Vincent Delecroix Ralf Stephan, Travis Scrimshaw N/A da8b056 da8b056e2d506516c80c19e2ffd002807310b533

Elements of positive characteristic in the symbolic ring are complete nonsense

```sage: x = SR.var('x')
sage: f(x) = exp(GF(3).one() * x)
sage: f
x |--> e^x
sage: bool(f(x) == exp(x))
True
sage: f(3*x)
1
```

or

```sage: solve(x^2 == 2, [x])
[x == -sqrt(2), x == sqrt(2)]
sage: solve(x^2 == GF(3)(2), [x])
[x == -sqrt(2), x == sqrt(2)]
```

More dramatically, it leads to segmentation faults

```sage: x = polygen(GF(3))
sage: a = SR.var('a')
sage: (2*x + 1) * a
segmentation fault
```

Even going through conversion

```sage: p = SR(2*x + 1)
sage: p * a
segmentation fault
```

We simply disallow wrapping of element of positive characteristic in `SR`.

Solve #18787 and #21391

Original reports for the segfault:

comment:1 Changed 4 years ago by dimpase

The real bug is that `SR(sigma)` succeeds (and returns something broken). It should just fail, and then `*` will fail too, as it should.

comment:2 Changed 4 years ago by jdemeyer

• Description modified (diff)
• Summary changed from Segfault when multiplication between symbolic expr. and non-monic poly over finite field to Segfault when multiplying symbolic expression and poly over finite field

comment:3 Changed 4 years ago by vdelecroix

• Description modified (diff)

comment:4 Changed 4 years ago by vdelecroix

• Authors set to Vincent Delecroix
• Branch set to u/vdelecroix/24072
• Status changed from new to needs_review

A branch where SR does not accept anymore positive characteristic... let see what the patchbot has to say.

New commits:

 ​148340c `24072: disallow positive characteristic in SR`

comment:5 Changed 4 years ago by git

• Commit changed from 148340c832900e841fea1ee25527383b26f607ad to 8f956e0ca57c8f15b94410c43e9a17bb534a7ced

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

 ​402865e `24072: fix doctest in matrix_space.py` ​8f956e0 `24072: fix doctests`

comment:6 Changed 4 years ago by vdelecroix

• Description modified (diff)
• Summary changed from Segfault when multiplying symbolic expression and poly over finite field to Disallow positive characteristic in the symbolic ring

comment:7 Changed 4 years ago by vdelecroix

I think I fixed all doctests!

comment:8 Changed 4 years ago by vdelecroix

• Description modified (diff)

comment:9 Changed 4 years ago by vdelecroix

• Description modified (diff)

comment:10 Changed 4 years ago by rws

• Reviewers set to Ralf Stephan
• Status changed from needs_review to positive_review

What a relief, thanks for your work.

comment:11 Changed 4 years ago by git

• Commit changed from 8f956e0ca57c8f15b94410c43e9a17bb534a7ced to de4e7607fe32aa771b2845cebcc784ab65a15236
• Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

 ​de4e760 `24072: py3 syntax`

comment:12 Changed 4 years ago by vdelecroix

• Status changed from needs_review to positive_review

A tiny change for py3 syntax (patchbot complaint). Setting back to positive review...

comment:13 Changed 4 years ago by dimpase

Does this mean that #21391 can now be closed as duplicate?

comment:14 Changed 4 years ago by vbraun

• Status changed from positive_review to needs_work

Merge conflict

comment:16 Changed 4 years ago by vdelecroix

• Description modified (diff)

comment:18 Changed 4 years ago by git

• Commit changed from de4e7607fe32aa771b2845cebcc784ab65a15236 to 3878ed6dcb385ec71dedb2dae28df53537418bc5

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

 ​32ffd59 `24072: disallow positive characteristic in SR` ​3878ed6 `24072: fix doctests`

comment:19 Changed 4 years ago by vdelecroix

• Status changed from needs_work to needs_review

rebased on 8.1.beta9

comment:20 Changed 4 years ago by git

• Commit changed from 3878ed6dcb385ec71dedb2dae28df53537418bc5 to da8b056e2d506516c80c19e2ffd002807310b533

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

 ​da8b056 `24072: fix doctests`

ping?

comment:22 Changed 4 years ago by tscrim

• Reviewers changed from Ralf Stephan to Ralf Stephan, Travis Scrimshaw
• Status changed from needs_review to positive_review

comment:23 Changed 4 years ago by vbraun

• Branch changed from u/vdelecroix/24072 to da8b056e2d506516c80c19e2ffd002807310b533
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.