Opened 18 months ago
Closed 16 months ago
#31864 closed enhancement (fixed)
Improved input output for backend polymake
Reported by:  ghkliem  Owned by:  

Priority:  critical  Milestone:  sage9.4 
Component:  geometry  Keywords:  polymake, input, output 
Cc:  Matthias Köppe  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  e6fef55 (Commits, GitHub, GitLab)  Commit:  e6fef555f7dfcf2418eb4398c66292ade2c494ce 
Dependencies:  #27745  Stopgaps: 
Description (last modified by )
From #26368.
The interface to polymake seems to have a significant delay. For input, we can avoid it, as polymake accepts nested lists of integers, rationals, floats.
For output we can avoid it, as at least for matrices and vectors, we can just parse the representation string.
Before:
sage: %time P = polytopes.hypercube(8, backend='polymake') CPU times: user 4.54 s, sys: 260 ms, total: 4.8 s Wall time: 4.81 s sage: %time P = polytopes.hypercube(8, backend='polymake') CPU times: user 3.08 s, sys: 236 ms, total: 3.31 s Wall time: 3.31 s sage: %time P1 = loads(dumps(P)) CPU times: user 805 ms, sys: 48 ms, total: 853 ms Wall time: 853 ms sage: %time P = polytopes.dodecahedron(backend='polymake') CPU times: user 763 ms, sys: 48.2 ms, total: 811 ms Wall time: 811 ms sage: %time P = polytopes.dodecahedron(backend='polymake') CPU times: user 661 ms, sys: 31.7 ms, total: 692 ms Wall time: 692 ms sage: %time P1 = loads(dumps(P)) CPU times: user 62.9 ms, sys: 0 ns, total: 62.9 ms Wall time: 62.6 ms sage: %time P = polytopes.dodecahedron(backend='polymake', exact=False) CPU times: user 408 ms, sys: 19.6 ms, total: 428 ms Wall time: 427 ms
After:
sage: %time P = polytopes.hypercube(8, backend='polymake') CPU times: user 1.56 s, sys: 47.4 ms, total: 1.61 s Wall time: 1.62 s sage: %time P = polytopes.hypercube(8, backend='polymake') CPU times: user 69.1 ms, sys: 4.07 ms, total: 73.2 ms Wall time: 72.8 ms sage: %time P1 = loads(dumps(P)) CPU times: user 36.5 ms, sys: 30 µs, total: 36.5 ms Wall time: 36 ms sage: %time P = polytopes.dodecahedron(backend='polymake') CPU times: user 206 ms, sys: 4.1 ms, total: 210 ms Wall time: 209 ms sage: %time P = polytopes.dodecahedron(backend='polymake') CPU times: user 67.9 ms, sys: 0 ns, total: 67.9 ms Wall time: 67.2 ms sage: %time P1 = loads(dumps(P)) CPU times: user 39.2 ms, sys: 3.7 ms, total: 42.9 ms Wall time: 41.8 ms sage: %time P = polytopes.dodecahedron(backend='polymake', exact=False) CPU times: user 78.3 ms, sys: 4.01 ms, total: 82.3 ms Wall time: 81.3 ms
Change History (13)
comment:1 Changed 18 months ago by
Status:  new → needs_review 

comment:2 Changed 18 months ago by
Commit:  4a663578407c7e00ea6cad4e65a6c9ca7dbf4600 → 543d2115d88cd57dc6939567982840999f20cd0d 

comment:3 Changed 18 months ago by
Description:  modified (diff) 

comment:4 Changed 18 months ago by
I think it would be better to call NumberFieldElement_quadratic._polymake_init_
instead of duplicating it
comment:6 Changed 18 months ago by
Commit:  543d2115d88cd57dc6939567982840999f20cd0d → 2f818be1be8286fb7e7692acd03eeb3cf99a5eff 

Branch pushed to git repo; I updated commit sha1. New commits:
cd6212d  expose memoryallocator as extra package

b2c75d8  replace by memory_allocator package

4d8927e  Merge branch 'u/ghkliem/outsource_memory_allocator' of git://trac.sagemath.org/sage into u/ghkliem/outsource_memory_allocator2

4589081  updated to newest beta

fd87c95  31866: fixing 3 doctests

44a0df4  Merge branch 'u/slabbe/31866' of git://trac.sagemath.org/sage into u/ghkliem/outsource_memory_allocator

ef9f3b2  Merge branch 'u/ghkliem/polymake_input_output' of git://trac.sagemath.org/sage into u/ghkliem/polymake_input_output_reb

2f818be  use `_polymake_init_` for quadratic fields

comment:8 Changed 18 months ago by
Branch:  u/ghkliem/polymake_input_output → u/ghkliem/polymake_input_output2 

Commit:  2f818be1be8286fb7e7692acd03eeb3cf99a5eff → e6fef555f7dfcf2418eb4398c66292ade2c494ce 
comment:9 Changed 18 months ago by
comment:10 Changed 18 months ago by
Reviewers:  → Matthias Koeppe 

Status:  needs_review → positive_review 
This is a very nice improvement.
comment:11 Changed 18 months ago by
Thank you. Sometimes it is surprising what the bottle neck of a calculation is.
comment:12 Changed 17 months ago by
Priority:  major → critical 

comment:13 Changed 16 months ago by
Branch:  u/ghkliem/polymake_input_output2 → e6fef555f7dfcf2418eb4398c66292ade2c494ce 

Resolution:  → fixed 
Status:  positive_review → closed 
Note: See
TracTickets for help on using
tickets.
Branch pushed to git repo; I updated commit sha1. New commits:
fix number field input