Opened 8 months ago

Closed 7 months ago

#30435 closed enhancement (fixed)

Improve count vertices of combinatorial faces

Reported by: gh-kliem Owned by:
Priority: minor Milestone: sage-9.2
Component: geometry Keywords: combinatorial polyhedron, popcount
Cc: tscrim Merged in:
Authors: Jonathan Kliem Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: bf91483 (Commits, GitHub, GitLab) Commit: bf91483e0aea5b5ad8904162124223f29450e687
Dependencies: #30428 Stopgaps:

Status badges

Description

From ​https://trac.sagemath.org/ticket/27103#comment:10

A small improvement that speeds up the current popcount.

Before: 

sage: P = polytopes.permutahedron(8, backend='field')                                                                                                                                                                                                                                                                                                                      
sage: C = CombinatorialPolyhedron(P)                                                                                                                                                                                                                                                                                                                                       
sage: %time C.is_simple()                                                                                                                                                                                                                                                                                                                                                  
CPU times: user 1.9 ms, sys: 82 µs, total: 1.99 ms
Wall time: 1.99 ms
True

With this ticket: 

sage: C = CombinatorialPolyhedron(P)                                                                                                                                                                                                                                                                                                                                       
sage: %time C.is_simple()                                                                                                                                                                                                                                                                                                                                                  
CPU times: user 334 µs, sys: 0 ns, total: 334 µs
Wall time: 336 µs
True

Eventually combinatorial polyhedron should use data_structures/bitset.pxi, but not before #27122 is merged.

Change History (4)

comment:1 Changed 8 months ago by gh-kliem

  • Branch set to u/gh-kliem/improved_count_atoms
  • Commit set to bf91483e0aea5b5ad8904162124223f29450e687
  • Dependencies set to #30428
  • Status changed from new to needs_review

New commits:

​39f3a38directly check is_simple/is_simplicial
​bf91483improved count atoms

comment:2 Changed 8 months ago by tscrim

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

LGTM.

comment:3 Changed 8 months ago by gh-kliem

Thank you.

comment:4 Changed 7 months ago by vbraun

  • Branch changed from u/gh-kliem/improved_count_atoms to bf91483e0aea5b5ad8904162124223f29450e687
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.