Opened 5 years ago
Closed 5 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: |
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: Meta-ticket: polymake
Change History (10)
comment:1 Changed 5 years ago by
- Branch set to u/mkoeppe/backend_polymake__work_around_polymake_bug_with_zero_inequalities_over_quadratic_extensions
comment:2 Changed 5 years ago by
- Commit set to 4070c66b4b1bb69db38df205ec72913a0d6c3407
- Dependencies set to #22683
comment:3 Changed 5 years ago by
- Description modified (diff)
comment:4 Changed 5 years ago by
- Status changed from new to needs_review
comment:5 Changed 5 years ago by
- Cc tscrim added
comment:6 Changed 5 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 code-wise.
comment:7 Changed 5 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 5 years ago by
Thanks. I agree. I've simplified the code. Also merged the updated #22658.
comment:9 Changed 5 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM. Thanks.
comment:10 Changed 5 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