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

backend_polymake: Work around polymake bug with zero inequalities over quadratic extensions #22723

Closed
mkoeppe opened this issue Mar 30, 2017 · 13 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Mar 30, 2017

Following up on #22683:

Easy workaround for a segfault, as discussed and suggested at
https://forum.polymake.org/viewtopic.php?f=8&t=547

See also: #22710: Meta-ticket: polymake

Depends on #22683

CC: @tscrim

Component: geometry

Keywords: polymake

Author: Matthias Koeppe

Branch/Commit: 93af258

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-8.0 milestone Mar 30, 2017
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 30, 2017

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 30, 2017

Commit: 4070c66

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 30, 2017

Dependencies: #22683

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 30, 2017

Last 10 new commits:

ef23af4Polyhedron_base._polymake_init_: Add test for RDF polyhedra
205879fMatrix, MatrixSpace: Add coercion to polymake interface
e49765ePolymake: Fix tests whose error messages changed because we do not use files
51af468PolymakeElement.__iter__: New
2f0e059PolymakeElement._sage_: Implement for *Vector, *Matrix, QuadraticExtension
218dacaAdd Polyhedron_polymake
5ab94a2Merge tag '7.6' into t/22683/backend_polymake_for_polyhedron
8fd81c7Polyhedron_polymake._polymake_: Add doctest
952b860PolymakeElement.__iter__: Doc fixes
4070c66Polyhedron_polymake: Work around polymake bug with zero inequalities over quadratic extensions

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 31, 2017

Author: Matthias Koeppe

@tscrim
Copy link
Collaborator

tscrim commented Apr 1, 2017

comment:6

Your check doesn't result in any speedup when the list passes as it has to check every element and has to double check when it fails. I think you're better off just running the filter directly:

ieqs = [ v for v in list if not all(self._is_zero(x) for x in v) ]

The other option would be a more hybrid approach like this:

for i,v in enumerate(ieqs):
    if all(self._is_zero(x) for x in v):
        ieqs = ieqs[:i] + [v for v in ieqs[i+1:] if all(self._is_zero(x) for x in v))
        break

Although I think always filtering is best because it is the simplest code-wise.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2017

Changed commit from 4070c66 to 93af258

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2017

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

8812e9cMerge tag '7.6' into t/22658/polyhedron_methods_using_the_polymake_pexpect_interface
b47f6f9Polyhedron_base._polymake_init_: Add work around for empty polytopes with Polymake 3.0
3b49a54Polymake: Fix another test whose error message depends on whether we use files
07a1757Polyhedron_base._polymake_init_: Fix comment
09942e7coercion tutorial: Update introspection results
19d05f1Add more 'optional - polymake
cd583e2Remove one unneccessary 'optional - polymake
7bcc04cMerge tag '8.0.beta0' into t/22658/polyhedron_methods_using_the_polymake_pexpect_interface
d7d4234Add '# optional - polymake'
93af258Merge branch 't/22658/polyhedron_methods_using_the_polymake_pexpect_interface' into t/22723/backend_polymake__work_around_polymake_bug_with_zero_inequalities_over_quadratic_extensions

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 1, 2017

comment:8

Thanks. I agree. I've simplified the code.
Also merged the updated #22658.

@tscrim
Copy link
Collaborator

tscrim commented Apr 1, 2017

comment:9

LGTM. Thanks.

@tscrim
Copy link
Collaborator

tscrim commented Apr 1, 2017

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Apr 5, 2017

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

3 participants