Opened 11 months ago

Closed 6 months ago

#30445 closed enhancement (fixed)

Merge duplications in edges, ridges and f-vector of combinatorial polyhedron

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.3
Component: geometry Keywords: code duplication, combinatorial polyhedron
Cc: jipilab, gh-LaisRast, tscrim Merged in:
Authors: Jonathan Kliem Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 62b3d9f (Commits, GitHub, GitLab) Commit: 62b3d9f0eaf22fc9102f990f4c7f3754c1282f79
Dependencies: #30443 Stopgaps:

Status badges

Description

We merge some duplications of obtaining edges, ridges and f-vector.

There was an entire copy of getting edges/ridges that also obtained the f-vector. But it is much simpler doing this directly in the f-vector code.

Also getting the edges/ridges is almost the same thing. We can let the FaceIterator worry about the details.

Change History (7)

comment:1 Changed 11 months ago by gh-kliem

  • Status changed from new to needs_review

comment:2 Changed 9 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:3 Changed 7 months ago by gh-kliem

  • Cc jipilab gh-LaisRast tscrim added

comment:4 Changed 7 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

Some minor formatting things (which you can either change or ignore and then set this to a positive review):

-    cdef size_t _compute_edges_or_ridges_with_iterator(self, FaceIterator face_iter, bint do_atom_rep, size_t ***edges_pt, size_t *counter_pt, size_t *current_length_pt, MemoryAllocator mem) except -1:
+    cdef size_t _compute_edges_or_ridges_with_iterator(self, FaceIterator face_iter,
+                                                       bint do_atom_rep,
+                                                       size_t ***edges_pt,
+                                                       size_t *counter_pt,
+                                                       size_t *current_length_pt,
+                                                       MemoryAllocator mem) except -1:
         r"""
         See :meth:`CombinatorialPolyhedron._compute_edges`.
         """
         cdef size_t a,b                # facets of an edge
         cdef int dim = self.dimension()
         cdef output_dimension = 1 if do_atom_rep else dim - 2
 
-        while (face_iter.next_dimension() == output_dimension):
+        while face_iter.next_dimension() == output_dimension:

comment:5 Changed 7 months ago by git

  • Commit changed from 8814532248adab90b73c4335bcaf37848622a28e to 62b3d9f0eaf22fc9102f990f4c7f3754c1282f79

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

cbc711dMerge branch 'u/gh-kliem/merge_edges_ridges_f_vector' of git://trac.sagemath.org/sage into u/gh-kliem/merge_edges_ridges_f_vector
62b3d9fformating

comment:6 Changed 7 months ago by gh-kliem

  • Status changed from needs_review to positive_review

Thank you.

comment:7 Changed 6 months ago by vbraun

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