Opened 2 years ago
Closed 21 months ago
#30524 closed enhancement (fixed)
Remove `maybe_newfaces` in combinatorial polyhedron
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  geometry  Keywords:  simplification, combinatorial polyhedron 
Cc:  tscrim  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fc8124a (Commits, GitHub, GitLab)  Commit:  fc8124a4986660d38297bab6723860d9bf99ed8c 
Dependencies:  #30040  Stopgaps: 
Description (last modified by )
This ticket removes maybe_newfaces
from the face iterator.
There used to be a uint64_t**
with the data called maybe_newfaces
and a uint64_t**
that points to the actual newfaces. We remove this double structure by swapping the new faces in place.
We also get rid of a superfluous function get_next_dimension_loop
in ListOfFaces
.
We do not expose the original pointers to the coatoms such that e.g. sorting of the coatoms would be possible in the face iterator (note that still the actual data is exposed, which one shouldn't alter anyway).
Change History (10)
comment:1 Changed 2 years ago by
 Branch set to u/ghkliem/no_more_maybes
 Commit set to 6d9f9c4067dfc97ef2f51df0509e28ac7a0cef3f
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit changed from 6d9f9c4067dfc97ef2f51df0509e28ac7a0cef3f to c91ae5bd4a28f3882697910167431b0d73e06f94
Branch pushed to git repo; I updated commit sha1. New commits:
c91ae5b  do not expose the coatoms

comment:3 Changed 2 years ago by
 Dependencies set to #30040
New commits:
c91ae5b  do not expose the coatoms

comment:4 Changed 2 years ago by
 Commit changed from c91ae5bd4a28f3882697910167431b0d73e06f94 to c16d4931d65fd6de5df9c24efb648534b98a1bb5
Branch pushed to git repo; I updated commit sha1. New commits:
c16d493  fix mistake

comment:5 Changed 2 years ago by
 Description modified (diff)
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:6 Changed 2 years ago by
Thank you.
comment:7 Changed 2 years ago by
 Commit changed from c16d4931d65fd6de5df9c24efb648534b98a1bb5 to fc8124a4986660d38297bab6723860d9bf99ed8c
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. Last 10 new commits:
c51fe62  simplify `get_next_level_simple by 30458

f3e2ddc  prepare slightly modified algorithm for simple/simplicial polytopes

edad681  typo

5939b5d  faster algorithm for simple/simplicial polytopes

f997cec  small fixes

63b7bd6  improvements in documentation

f2edc3d  get rid of maybe_newfaces

6dfbe81  get rid of compute_dimension_loop

f2dd072  do not expose the coatoms

fc8124a  fix mistake

comment:9 Changed 22 months ago by
 Milestone changed from sage9.2 to sage9.3
comment:10 Changed 21 months ago by
 Branch changed from u/ghkliem/no_more_maybes to fc8124a4986660d38297bab6723860d9bf99ed8c
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
prepare slightly modified algorithm for simple/simplicial polytopes
typo
faster algorithm for simple/simplicial polytopes
small fixes
improvements in documentation
Merge branch 'u/ghkliem/improved_count_atoms' of git://trac.sagemath.org/sage into public/30040reb2
Merge branch 'develop' of git://trac.sagemath.org/sage into public/30040reb2
Merge branch 'public/30040reb2' of git://trac.sagemath.org/sage into u/ghkliem/hide_data_structure_a_bit
get rid of maybe_newfaces
get rid of compute_dimension_loop