Opened 2 years ago

Closed 21 months ago

#30524 closed enhancement (fixed)

Remove `maybe_newfaces` in combinatorial polyhedron

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.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:

Status badges

Description (last modified by tscrim)

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 gh-kliem

  • Branch set to u/gh-kliem/no_more_maybes
  • Commit set to 6d9f9c4067dfc97ef2f51df0509e28ac7a0cef3f
  • Status changed from new to needs_review

Last 10 new commits:

​9cece74prepare slightly modified algorithm for simple/simplicial polytopes
​fe5852atypo
​1510386faster algorithm for simple/simplicial polytopes
​86c564asmall fixes
​ed6e966improvements in documentation
​40e6667Merge branch 'u/gh-kliem/improved_count_atoms' of git://trac.sagemath.org/sage into public/30040-reb2
​242cba7Merge branch 'develop' of git://trac.sagemath.org/sage into public/30040-reb2
​41c4541Merge branch 'public/30040-reb2' of git://trac.sagemath.org/sage into u/gh-kliem/hide_data_structure_a_bit
​25a1bafget rid of maybe_newfaces
​6d9f9c4get rid of compute_dimension_loop

comment:2 Changed 2 years ago by git

  • Commit changed from 6d9f9c4067dfc97ef2f51df0509e28ac7a0cef3f to c91ae5bd4a28f3882697910167431b0d73e06f94

Branch pushed to git repo; I updated commit sha1. New commits:

​c91ae5bdo not expose the coatoms

comment:3 Changed 2 years ago by gh-kliem

  • Dependencies set to #30040

New commits:

​c91ae5bdo not expose the coatoms

comment:4 Changed 2 years ago by git

  • Commit changed from c91ae5bd4a28f3882697910167431b0d73e06f94 to c16d4931d65fd6de5df9c24efb648534b98a1bb5

Branch pushed to git repo; I updated commit sha1. New commits:

​c16d493fix mistake

comment:5 Changed 2 years ago by tscrim

  • Description modified (diff)
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:6 Changed 2 years ago by gh-kliem

Thank you.

comment:7 Changed 2 years ago by git

  • 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:

​c51fe62simplify `get_next_level_simple by 30458
​f3e2ddcprepare slightly modified algorithm for simple/simplicial polytopes
​edad681typo
​5939b5dfaster algorithm for simple/simplicial polytopes
​f997cecsmall fixes
​63b7bd6improvements in documentation
​f2edc3dget rid of maybe_newfaces
​6dfbe81get rid of compute_dimension_loop
​f2dd072do not expose the coatoms
​fc8124afix mistake

comment:8 Changed 2 years ago by gh-kliem

  • Status changed from needs_review to positive_review

Rebased.

comment:9 Changed 22 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:10 Changed 21 months ago by vbraun

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