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

Inheriting from MPolynomialRing_libsingular crashes sage #26958

Closed
Etn40ff opened this issue Dec 25, 2018 · 11 comments
Closed

Inheriting from MPolynomialRing_libsingular crashes sage #26958

Etn40ff opened this issue Dec 25, 2018 · 11 comments

Comments

@Etn40ff
Copy link
Contributor

Etn40ff commented Dec 25, 2018

The following crashes sage with a SIGSEGV:

sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
sage: class Foo(MPolynomialRing_libsingular):
....:     pass
sage: Foo(QQ, 2, ['x','y'], 'degrevlex')

Strangely enough this bug is triggered only when inheriting; indeed the following works as expected:

sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
sage: MPolynomialRing_libsingular(QQ, 2, ['x','y'], 'degrevlex')
Multivariate Polynomial Ring in x, y over Rational Field

As it turns out this bug is due to a spurious call to __init_extra__ of Algebras(...).parent_class. Univariate polynomial rings avoid this call by setting the attribute _no_generic_basering_coercion = True; now multivariate polynomial rings do so too.

Component: algebra

Keywords: polynomial rings

Author: Salvatore Stella

Branch/Commit: 57c8182

Reviewer: Jeroen Demeyer

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

@Etn40ff Etn40ff added this to the sage-8.6 milestone Dec 25, 2018
@Etn40ff
Copy link
Contributor Author

Etn40ff commented Dec 25, 2018

New commits:

d628f14Fix 26958

@Etn40ff
Copy link
Contributor Author

Etn40ff commented Dec 25, 2018

Branch: u/etn40ff/26958

@Etn40ff
Copy link
Contributor Author

Etn40ff commented Dec 25, 2018

Commit: d628f14

@jdemeyer
Copy link

comment:2

Doesn't hurt I suppose...

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

comment:3

Although maybe a minor comment about the code: can you replace

cdef public bool _no_generic_basering_coercion

by

cdef readonly _no_generic_basering_coercion

and remove the bool cimport (which serves no purpose)?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2018

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

57c8182Avoid importing bool

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2018

Changed commit from d628f14 to 57c8182

@Etn40ff
Copy link
Contributor Author

Etn40ff commented Dec 27, 2018

comment:5

Done, thanks for the review.

A side note worth noting for #25558: after this patch it is possible to inherit from MPolynomialRing_libsingular but it is not possible to change its element class because the element constructor uses a function, new_MP, which hardcodes its output.

@embray
Copy link
Contributor

embray commented Jan 15, 2019

comment:8

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

@embray embray modified the milestones: sage-8.6, sage-8.7 Jan 15, 2019
@vbraun
Copy link
Member

vbraun commented Jan 23, 2019

Changed branch from u/etn40ff/26958 to 57c8182

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

4 participants