Opened 3 years ago

Closed 3 years ago

#24072 closed defect (fixed)

Disallow positive characteristic in the symbolic ring

Reported by: bruno Owned by:
Priority: critical Milestone: sage-8.1
Component: commutative algebra Keywords: polynomial, symbolics
Cc: Merged in:
Authors: Vincent Delecroix Reviewers: Ralf Stephan, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: da8b056 (Commits) Commit: da8b056e2d506516c80c19e2ffd002807310b533
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

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:

Change History (23)

comment:1 Changed 3 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 3 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 3 years ago by vdelecroix

  • Description modified (diff)
  • Keywords symbolics added

comment:4 Changed 3 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Branch set to u/vdelecroix/24072
  • Commit set to 148340c832900e841fea1ee25527383b26f607ad
  • 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:

​148340c24072: disallow positive characteristic in SR

comment:5 Changed 3 years ago by git

  • Commit changed from 148340c832900e841fea1ee25527383b26f607ad to 8f956e0ca57c8f15b94410c43e9a17bb534a7ced

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

​402865e24072: fix doctest in matrix_space.py
​8f956e024072: fix doctests

comment:6 Changed 3 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 3 years ago by vdelecroix

I think I fixed all doctests!

comment:8 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:9 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:10 Changed 3 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 3 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:

​de4e76024072: py3 syntax

comment:12 Changed 3 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 3 years ago by dimpase

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

comment:14 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:15 Changed 3 years ago by vdelecroix

Volker, can you provide more information?

comment:16 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:17 Changed 3 years ago by dimpase

Hmm, I guess, look at ​https://github.com/vbraun/sage/tree/develop ?

comment:18 Changed 3 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:

​32ffd5924072: disallow positive characteristic in SR
​3878ed624072: fix doctests

comment:19 Changed 3 years ago by vdelecroix

  • Status changed from needs_work to needs_review

rebased on 8.1.beta9

comment:20 Changed 3 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:

​da8b05624072: fix doctests

comment:21 Changed 3 years ago by vdelecroix

ping?

comment:22 Changed 3 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 3 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.