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: sage-8.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:

Status badges

Description (last modified by mkoeppe)

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

Change History (10)

comment:1 Changed 4 years ago by mkoeppe

  • Branch set to u/mkoeppe/backend_polymake__work_around_polymake_bug_with_zero_inequalities_over_quadratic_extensions

comment:2 Changed 4 years ago by mkoeppe

  • Commit set to 4070c66b4b1bb69db38df205ec72913a0d6c3407
  • Dependencies set to #22683

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

comment:3 Changed 4 years ago by mkoeppe

  • Description modified (diff)

comment:4 Changed 4 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Status changed from new to needs_review

comment:5 Changed 4 years ago by mkoeppe

  • Cc tscrim added

comment:6 Changed 4 years ago by tscrim

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.

comment:7 Changed 4 years ago by git

  • Commit changed from 4070c66b4b1bb69db38df205ec72913a0d6c3407 to 93af25844a4a55c5372bfb55d896784de2945db8

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

comment:8 Changed 4 years ago by mkoeppe

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

comment:9 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM. Thanks.

comment:10 Changed 4 years ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.