Opened 4 years ago
Closed 4 years ago
#22723 closed defect (fixed)
backend_polymake: Work around polymake bug with zero inequalities over quadratic extensions
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  geometry  Keywords:  polymake 
Cc:  tscrim  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  93af258 (Commits, GitHub, GitLab)  Commit:  93af25844a4a55c5372bfb55d896784de2945db8 
Dependencies:  #22683  Stopgaps: 
Description (last modified by )
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: Metaticket: polymake
Change History (10)
comment:1 Changed 4 years ago by
 Branch set to u/mkoeppe/backend_polymake__work_around_polymake_bug_with_zero_inequalities_over_quadratic_extensions
comment:2 Changed 4 years ago by
 Commit set to 4070c66b4b1bb69db38df205ec72913a0d6c3407
 Dependencies set to #22683
comment:3 Changed 4 years ago by
 Description modified (diff)
comment:4 Changed 4 years ago by
 Status changed from new to needs_review
comment:5 Changed 4 years ago by
 Cc tscrim added
comment:6 Changed 4 years ago by
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 codewise.
comment:7 Changed 4 years ago by
 Commit changed from 4070c66b4b1bb69db38df205ec72913a0d6c3407 to 93af25844a4a55c5372bfb55d896784de2945db8
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
8812e9c  Merge tag '7.6' into t/22658/polyhedron_methods_using_the_polymake_pexpect_interface

b47f6f9  Polyhedron_base._polymake_init_: Add work around for empty polytopes with Polymake 3.0

3b49a54  Polymake: Fix another test whose error message depends on whether we use files

07a1757  Polyhedron_base._polymake_init_: Fix comment

09942e7  coercion tutorial: Update introspection results

19d05f1  Add more 'optional  polymake

cd583e2  Remove one unneccessary 'optional  polymake

7bcc04c  Merge tag '8.0.beta0' into t/22658/polyhedron_methods_using_the_polymake_pexpect_interface

d7d4234  Add '# optional  polymake'

93af258  Merge 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

comment:8 Changed 4 years ago by
Thanks. I agree. I've simplified the code. Also merged the updated #22658.
comment:9 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM. Thanks.
comment:10 Changed 4 years ago by
 Branch changed from u/mkoeppe/backend_polymake__work_around_polymake_bug_with_zero_inequalities_over_quadratic_extensions to 93af25844a4a55c5372bfb55d896784de2945db8
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
Polyhedron_base._polymake_init_: Add test for RDF polyhedra
Matrix, MatrixSpace: Add coercion to polymake interface
Polymake: Fix tests whose error messages changed because we do not use files
PolymakeElement.__iter__: New
PolymakeElement._sage_: Implement for *Vector, *Matrix, QuadraticExtension
Add Polyhedron_polymake
Merge tag '7.6' into t/22683/backend_polymake_for_polyhedron
Polyhedron_polymake._polymake_: Add doctest
PolymakeElement.__iter__: Doc fixes
Polyhedron_polymake: Work around polymake bug with zero inequalities over quadratic extensions