Opened 2 years ago

Closed 23 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: Jean-Philippe Labbé, Laith Rastanawi, Travis Scrimshaw 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 2 years ago by gh-kliem

Status: newneeds_review

comment:2 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:3 Changed 2 years ago by gh-kliem

Cc: Jean-Philippe Labbé Laith Rastanawi Travis Scrimshaw added

comment:4 Changed 2 years ago by Travis Scrimshaw

Reviewers: 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 2 years ago by git

Commit: 8814532248adab90b73c4335bcaf37848622a28e62b3d9f0eaf22fc9102f990f4c7f3754c1282f79

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 2 years ago by gh-kliem

Status: needs_reviewpositive_review

Thank you.

comment:7 Changed 23 months ago by Volker Braun

Branch: u/gh-kliem/merge_edges_ridges_f_vector62b3d9f0eaf22fc9102f990f4c7f3754c1282f79
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.