Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outsource some functions in bit_vector_operations.cc #30458

Closed
kliem opened this issue Aug 28, 2020 · 17 comments
Closed

Outsource some functions in bit_vector_operations.cc #30458

kliem opened this issue Aug 28, 2020 · 17 comments

Comments

@kliem
Copy link
Contributor

kliem commented Aug 28, 2020

We simplify obtaining the inclusion maximal faces in the function
get_next_level by outsourcing to new function is_contained_in_one.

In addition we implement contains_one for #30040.

This ticket merges cleanly with #30435.

Component: geometry

Keywords: code duplication, combinatorial polyhedron

Author: Jonathan Kliem

Branch/Commit: 5311cdf

Reviewer: Travis Scrimshaw, Samuel Lelièvre

Issue created by migration from https://trac.sagemath.org/ticket/30458

@kliem kliem added this to the sage-9.2 milestone Aug 28, 2020
@kliem
Copy link
Contributor Author

kliem commented Aug 28, 2020

@kliem
Copy link
Contributor Author

kliem commented Aug 28, 2020

New commits:

20a39b6outsource inclusion maximal

@kliem
Copy link
Contributor Author

kliem commented Aug 28, 2020

Commit: 20a39b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2020

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

b672fcaremoved redundant function

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2020

Changed commit from 20a39b6 to b672fca

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2020

comment:3

LGTM.

@kliem
Copy link
Contributor Author

kliem commented Aug 30, 2020

comment:4

Thank you.

@slel
Copy link
Member

slel commented Aug 30, 2020

comment:5

The functions inline int is_contained_in_one (with or without skip)
have this fine description:

     Return whether ``face`` is contained in one of ``faces``.

Descriptions for the functions contains_one (with or without skip)
have a problem:

 inline int contains_one(uint64_t *face, uint64_t **faces, size_t n_faces, size_t face_length){
     /*
-    Return whether ``face`` contains in one of ``faces``.
+    Return whether ``face`` contains one of ``faces``.
     */
 inline int contains_one(uint64_t *face, uint64_t **faces, size_t n_faces, size_t face_length, size_t skip){
     /*
-    Return whether ``face`` is contains in one of ``faces``.
+    Return whether ``face`` contains one of ``faces``.
 
     Skips ``faces[skip]``.
     */

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2020

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

b2ce23egrammar

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2020

Changed commit from b672fca to b2ce23e

@slel
Copy link
Member

slel commented Aug 30, 2020

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Samuel Lelièvre

@kliem
Copy link
Contributor Author

kliem commented Aug 30, 2020

comment:9

Thanks again.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

7c970e3outsource inclusion maximal
92fb29cremoved redundant function
5311cdfgrammar

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 9, 2020

Changed commit from b2ce23e to 5311cdf

@kliem
Copy link
Contributor Author

kliem commented Sep 9, 2020

comment:11

Rebased.

@vbraun
Copy link
Member

vbraun commented Nov 7, 2020

Changed branch from u/gh-kliem/outsource_inclusion_maximal to 5311cdf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants