Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow positive characteristic in the symbolic ring #24072

Closed
bgrenet opened this issue Oct 20, 2017 · 32 comments
Closed

Disallow positive characteristic in the symbolic ring #24072

bgrenet opened this issue Oct 20, 2017 · 32 comments

Comments

@bgrenet
Copy link

bgrenet commented Oct 20, 2017

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:

Component: commutative algebra

Keywords: polynomial, symbolics

Author: Vincent Delecroix

Branch/Commit: da8b056

Reviewer: Ralf Stephan, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/24072

@dimpase
Copy link
Member

dimpase commented Oct 20, 2017

comment:1

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

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Segfault when multiplication between symbolic expr. and non-monic poly over finite field Segfault when multiplying symbolic expression and poly over finite field Oct 20, 2017
@videlec
Copy link
Contributor

videlec commented Oct 20, 2017

Changed keywords from polynomial to polynomial, symbolics

@videlec

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

Author: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

Commit: 148340c

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

comment:4

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

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

Branch: u/vdelecroix/24072

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2017

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2017

Changed commit from 148340c to 8f956e0

@videlec

This comment has been minimized.

@videlec videlec changed the title Segfault when multiplying symbolic expression and poly over finite field Disallow positive characteristic in the symbolic ring Oct 21, 2017
@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

comment:7

I think I fixed all doctests!

@videlec

This comment has been minimized.

@videlec

This comment has been minimized.

@rwst
Copy link

rwst commented Oct 21, 2017

Reviewer: Ralf Stephan

@rwst
Copy link

rwst commented Oct 21, 2017

comment:10

What a relief, thanks for your work.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2017

Changed commit from 8f956e0 to de4e760

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2017

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

de4e76024072: py3 syntax

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

comment:12

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

@dimpase
Copy link
Member

dimpase commented Oct 21, 2017

comment:13

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

@vbraun
Copy link
Member

vbraun commented Oct 21, 2017

comment:14

Merge conflict

@videlec
Copy link
Contributor

videlec commented Oct 21, 2017

comment:15

Volker, can you provide more information?

@videlec

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Oct 21, 2017

comment:17

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 22, 2017

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 22, 2017

Changed commit from de4e760 to 3878ed6

@videlec
Copy link
Contributor

videlec commented Oct 22, 2017

comment:19

rebased on 8.1.beta9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 22, 2017

Changed commit from 3878ed6 to da8b056

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 22, 2017

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

da8b05624072: fix doctests

@videlec
Copy link
Contributor

videlec commented Oct 23, 2017

comment:21

ping?

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2017

Changed reviewer from Ralf Stephan to Ralf Stephan, Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Oct 29, 2017

Changed branch from u/vdelecroix/24072 to da8b056

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants